Add versioned doc support #905

zuochengding · 2021-11-03T04:16:22Z

Description

This PR is to add versioned doc url support in DocLinksServices.

My proposal is to use package.json to control the doc url versioning. If the branch value is main then convert it to latest as the document version, otherwise use the branch value as the document version (e.g. 1.1, or 1.0)
Create new doc link mappings based on doc website. Group them into three parts : opeansearch, opensearchDashboards and noDocuments. Mapping the structure into: [Group] -> [sub-page]->[sub-subpage] -> [anchor].
Fix DQL link related bug
Fix unit tests
Update URL links to latest version.

Issues Resolved

Fixed DQL Links Bug [BUG] DQL Link in Syntax Options goes to unrelated query language #769
[Docs] handle version in docs #892

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

zuochengding · 2021-11-03T04:28:27Z

Pipeline Testing:

tmarkley

I think this would be a good opportunity to clean up the DocLinksService a bit and make it clear which documentation does and does not exist. I believe in #335 we agreed to re-route "dead" links to the root documentation page for now, which we haven't done for all links, so we could fix that as well?

src/plugins/data/common/opensearch_query/kuery/node_types/wildcard.ts

src/plugins/data/server/autocomplete/value_suggestions_route.ts

src/core/public/doc_links/doc_links_service.ts

src/core/server/ui_settings/settings/date_formats.ts

src/core/public/doc_links/doc_links_service.ts

tmarkley · 2021-11-03T15:55:32Z

Looks like there are a few linting errors. You can fix those with node scripts/precommit_hook --fix and then commit the changes.

package.json

zuochengding · 2021-11-05T02:09:02Z

tmarkley

Looks good! Just a few minor questions/suggestions.

tmarkley · 2021-11-05T17:14:05Z

src/core/public/doc_links/doc_links_service.ts

+            // https://opensearch.org/docs/latest/opensearch/install/docker-security/
+            dockerSecurity: `${OPENSEARCH_VERSIONED_DOCS}install/docker-security`,
+            // https://opensearch.org/docs/latest/opensearch/install/helm/
+            helm: `${OPENSEARCH_VERSIONED_DOCS}install//helm/`,


typo (extra /)?

tmarkley · 2021-11-05T17:16:27Z

src/core/public/doc_links/doc_links_service.ts

-    };
-    readonly date: {
-      readonly dateMath: string;
+    noDocumentation: {


Is there a reason this isn't readonly?

Should mark as readonly as well

tmarkley · 2021-11-05T17:18:39Z

src/core/public/public.api.md

+      readonly management: Record<string, string>;
+      readonly visualize: Record<string, string>;


Do we know why these are different from the others?

Some unit test logic use different setting as a key to retrieve corresponding url. So i keep the Record type as it was.

tmarkley · 2021-11-05T17:18:56Z

src/core/public/public.api.md

+      readonly visualize: Record<string, string>;
+      readonly addData: string;
+    };
+    readonly scriptedFields: {


Doesn't this exist inside of noDocumentation?

Nice catch. I was changing the structure in doc_links_service but forgot to copy paste here

tmarkley · 2021-11-05T17:35:28Z

src/plugins/data/public/ui/query_string_input/language_switcher.tsx

-  const opensearchDashboards = useOpenSearchDashboards();
-  const kueryQuerySyntaxDocs = opensearchDashboards.services.docLinks!.links.query.kueryQuerySyntax;
+  const docLinks = useOpenSearchDashboards().services.docLinks;
+  const kueryQuerySyntaxDocs = docLinks!.links.opensearchDashboards.dql.base;


Should we take the opportunity to rename these keury variables now as referenced in #769?

will update

tmarkley · 2021-11-05T17:41:56Z

src/plugins/vis_type_vega/public/components/vega_help_menu.tsx

+  const opensearchDashboadsDoc = opensearchDashboards.services.docLinks!.links.opensearchDashboards
+    .introduction;


It might be better to add a vega property to the noDocumentation object?

Yes, we can.

package.json

src/core/public/public.api.md

kavilla · 2021-11-05T22:41:24Z

src/plugins/vis_default_editor/public/components/controls/date_ranges.tsx

@@ -141,7 +141,10 @@ function DateRangesParamEditor({
    <EuiFormRow compressed fullWidth>
      <>
        <EuiText size="xs">
-          <EuiLink href={services.docLinks.links.date.dateMath} target="_blank">
+          <EuiLink
+            href={services.docLinks.links.opensearch.aggregations.bucket.range}


is this the right link? bucket aggregations are a different thing.

Not so sure, I was thinking to use : https://opensearch.org/docs/latest/opensearch/units/ but it doesn't contains enough info related date_range. Instead, https://opensearch.org/docs/latest/opensearch/bucket-agg/#range-date_range-ip_range, contains more info about date_range. Open to any inputs

kavilla · 2021-11-08T22:29:32Z

Can this one be replaced by dql?

kavilla · 2021-11-08T22:41:54Z

Like the proposal, do we know if custom plugins consume the docs service? Like I would check some of the dashboards plugins in the opensearch-project and see if they are using some of the links provided. If so we might have to prep folks and wait for a 2.0 version bump.

Also, have you verified the 'zero-data' state works fine. I pulled this down and works fine with me just haven't checked zero data and thought I'd ask before clearing my stuff out.

NIT: Can you squash the commits with the context provided?

Thanks!

zuochengding · 2021-11-09T00:31:48Z

Yes, I have tested the app with no data ingested, the urls look fine to me. My understanding is that all the urls in DocLinksService are from external doc website, shouldn't have dependency on local data indexing.

To make sure my understanding regarding the comments is correct, you are suggesting to use :

https://opensearch.org/docs/latest/opensearch/query-dsl/index/ for Query string options
create a default dataMath url in noDocument group for date.dateMath?

Will squash commits in next revision

kavilla · 2021-11-09T00:42:40Z

Yes, I have tested the app with no data ingested, the urls look fine to me. My understanding is that all the urls in DocLinksService are from external doc website, shouldn't have dependency on local data indexing.

For sure, just making sure if there were links on views that only showed in the empty data state existed then there are still working.

https://opensearch.org/docs/latest/opensearch/query-dsl/index/ for Query string options

Not sure, if it should go to Query DSL, seeing more if it should go the newly created to the new DQL link created.

create an default dataMath url in noDocument group for date.dateMath
right?

If dateMath doesn't exist than 10-4 because just looking in the diff going from date to bucket agg that seems incorrect to me. If it was a bug that it was referencing date math but should have referenced aggs then so be it. But if its just a dead link then add it to the no docs section.

Will squash commits in next revision

💯

Any comment about custom plugins or still verifying that?

zuochengding · 2021-11-09T00:44:53Z

Need some time to check the plugins

zuochengding · 2021-11-09T01:29:27Z

By searching the code references in other open search plugin repos, there is no plugin depended on doc_links_service.ts. Most plugin projects just use hardcoded doc website links in their code base .

kavilla

Since adding a conditional we should probably had a test or two for verifying the conditional. Otherwise LGTM. I'm in an opinion this doesn't get backported to 1.2 and hesitatant to backport to 1.x since we are restructuring things. I don't think external plugins would be consuming this but just in case.

boktorbb

LGTM. I noticed we have the path src/plugins/data/common/opensearch_query/kuery. We should probably rename this. It doesn't have to be in this PR so we don't pollute the commits but it should probably be done

tmarkley · 2021-11-11T00:55:12Z

Since adding a conditional we should probably had a test or two for verifying the conditional. Otherwise LGTM. I'm in an opinion this doesn't get backported to 1.2 and hesitant to backport to 1.x since we are restructuring things. I don't think external plugins would be consuming this but just in case.

Agreed on all counts. Can we spend a little more time on adding tests?

zuochengding · 2021-11-11T02:18:40Z

Yeah. Working on adding the new tests

tmarkley · 2021-11-16T17:52:22Z

Like the proposal, do we know if custom plugins consume the docs service? Like I would check some of the dashboards plugins in the opensearch-project and see if they are using some of the links provided. If so we might have to prep folks and wait for a 2.0 version bump.

Is this the case for any service in our repo? If so, that's a serious inhibitor for any change that we're making. Not being able to refactor anything between major version bumps is a slippery slope.

tmarkley

This looks great! Just a couple very minor changes and then I think we're good to go.

src/plugins/vis_type_vega/public/components/vega_help_menu.tsx

src/plugins/data/public/ui/query_string_input/language_switcher.tsx

src/core/public/doc_links/doc_links_service.test.ts

This is PR is to add versioned document support in OSD. 1. Add logic to pick up doc version from package.json and convert it to `latest` if we are on default `main` branch. 2. Refactor doc_link_service to have 3 urls groups: opensearch, opensearchDashboards, and noDocumentation. 3. Update dynamic versioned doc links and clean up unused urls 4. Fix known url bug opensearch-project#769 5. Add unit tests for doclinks branch name conversion Signed-off-by: Zuocheng Ding <zding817@gmail.com>

kavilla

LGTM!

tmarkley

This is great!

…rch-project#905) bump semantic-release from 16.0.1 to 17.2.3 and upgrade semantic-release deps and node version Bumps [semantic-release](https://github.com/semantic-release/semantic-release) from 16.0.1 to 17.2.3. - [Release notes](https://github.com/semantic-release/semantic-release/releases) - [Commits](semantic-release/semantic-release@v16.0.1...v17.2.3) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>

zuochengding requested review from tmarkley, kavilla, boktorbb and ananzh November 3, 2021 04:16

zuochengding self-assigned this Nov 3, 2021

zuochengding added docs Improvements or additions to documentation rename-docs Document rename tracking enhancement New feature or request labels Nov 3, 2021

zuochengding linked an issue Nov 3, 2021 that may be closed by this pull request

[BUG] DQL Link in Syntax Options goes to unrelated query language #769

Closed

tmarkley suggested changes Nov 3, 2021

View reviewed changes

tmarkley reviewed Nov 3, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

zuochengding linked an issue Nov 5, 2021 that may be closed by this pull request

[Docs] handle version in docs #892

Closed

zuochengding requested a review from tmarkley November 5, 2021 02:09

tmarkley reviewed Nov 5, 2021

View reviewed changes

zuochengding requested a review from tmarkley November 5, 2021 18:53

kavilla reviewed Nov 5, 2021

View reviewed changes

zuochengding force-pushed the versioned-doc branch from 7242baf to 33cd5e8 Compare November 9, 2021 02:28

kavilla previously approved these changes Nov 9, 2021

View reviewed changes

boktorbb previously approved these changes Nov 10, 2021

View reviewed changes

zuochengding dismissed stale reviews from boktorbb and kavilla via a1f2927 November 11, 2021 17:58

zuochengding force-pushed the versioned-doc branch from 33cd5e8 to a1f2927 Compare November 11, 2021 17:58

zuochengding mentioned this pull request Nov 15, 2021

Broken link checker #938

Merged

5 tasks

zuochengding requested review from kavilla and boktorbb November 15, 2021 21:08

tmarkley reviewed Nov 16, 2021

View reviewed changes

src/plugins/vis_type_vega/public/components/vega_help_menu.tsx Outdated Show resolved Hide resolved

src/plugins/data/public/ui/query_string_input/language_switcher.tsx Outdated Show resolved Hide resolved

kavilla reviewed Nov 16, 2021

View reviewed changes

src/core/public/doc_links/doc_links_service.test.ts Outdated Show resolved Hide resolved

zuochengding force-pushed the versioned-doc branch from a1f2927 to 7c1c09a Compare November 17, 2021 17:58

zuochengding requested a review from a team as a code owner November 17, 2021 17:58

zuochengding force-pushed the versioned-doc branch from 7c1c09a to 6b5d287 Compare November 17, 2021 18:11

kavilla approved these changes Nov 17, 2021

View reviewed changes

tmarkley added the v1.3.0 label Nov 18, 2021

tmarkley approved these changes Nov 18, 2021

View reviewed changes

zuochengding merged commit 9047044 into opensearch-project:main Nov 18, 2021

zuochengding deleted the versioned-doc branch November 18, 2021 22:11

tmarkley added the backport pending label Nov 22, 2021

zuochengding mentioned this pull request Dec 22, 2021

[1.x] [Doc] Add versioned doc support and lychee link checker #1042

Merged

7 tasks

tmarkley mentioned this pull request Jan 4, 2022

[BUG] Replace documents with prod urls #335

Closed

kavilla removed the backport pending label Jan 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add versioned doc support #905

Add versioned doc support #905

zuochengding commented Nov 3, 2021 •

edited

Loading

zuochengding commented Nov 3, 2021

tmarkley left a comment

tmarkley commented Nov 3, 2021

zuochengding commented Nov 5, 2021

tmarkley left a comment

tmarkley Nov 5, 2021

zuochengding Nov 5, 2021

tmarkley Nov 5, 2021

zuochengding Nov 5, 2021

tmarkley Nov 5, 2021

zuochengding Nov 5, 2021

tmarkley Nov 5, 2021

zuochengding Nov 5, 2021

tmarkley Nov 5, 2021

zuochengding Nov 5, 2021

tmarkley Nov 5, 2021

zuochengding Nov 5, 2021

kavilla Nov 5, 2021

zuochengding Nov 5, 2021 •

edited

Loading

kavilla commented Nov 8, 2021

kavilla commented Nov 8, 2021

zuochengding commented Nov 9, 2021 •

edited

Loading

kavilla commented Nov 9, 2021

zuochengding commented Nov 9, 2021

zuochengding commented Nov 9, 2021

kavilla left a comment

boktorbb left a comment

tmarkley commented Nov 11, 2021

zuochengding commented Nov 11, 2021 •

edited

Loading

tmarkley commented Nov 16, 2021

tmarkley left a comment

kavilla left a comment

tmarkley left a comment

		readonly management: Record<string, string>;
		readonly visualize: Record<string, string>;

		const opensearchDashboadsDoc = opensearchDashboards.services.docLinks!.links.opensearchDashboards
		.introduction;

Add versioned doc support #905

Add versioned doc support #905

Conversation

zuochengding commented Nov 3, 2021 • edited Loading

Description

Issues Resolved

Check List

zuochengding commented Nov 3, 2021

tmarkley left a comment

Choose a reason for hiding this comment

tmarkley commented Nov 3, 2021

zuochengding commented Nov 5, 2021

tmarkley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zuochengding Nov 5, 2021 • edited Loading

Choose a reason for hiding this comment

kavilla commented Nov 8, 2021

kavilla commented Nov 8, 2021

zuochengding commented Nov 9, 2021 • edited Loading

kavilla commented Nov 9, 2021

zuochengding commented Nov 9, 2021

zuochengding commented Nov 9, 2021

kavilla left a comment

Choose a reason for hiding this comment

boktorbb left a comment

Choose a reason for hiding this comment

tmarkley commented Nov 11, 2021

zuochengding commented Nov 11, 2021 • edited Loading

tmarkley commented Nov 16, 2021

tmarkley left a comment

Choose a reason for hiding this comment

kavilla left a comment

Choose a reason for hiding this comment

tmarkley left a comment

Choose a reason for hiding this comment

zuochengding commented Nov 3, 2021 •

edited

Loading

zuochengding Nov 5, 2021 •

edited

Loading

zuochengding commented Nov 9, 2021 •

edited

Loading

zuochengding commented Nov 11, 2021 •

edited

Loading