Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adding version switcher to Triton sphinx documentation #7872

Closed
wants to merge 6 commits into from

Conversation

nv-kmcgill53
Copy link
Contributor

@nv-kmcgill53 nv-kmcgill53 commented Dec 11, 2024

What does the PR do?

This PR adds a switcher to pick different versions of Triton documentation. These changes:

  1. Build the switcher data. For now, the list includes the last 12 releases (1 year) using the major/minor/patch convention, not the year/month convention. This can be changed in the future, or added depending on development time.
  2. Validates the final URLs which are to be added to the switcher.

image

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs:

None

Where should the reviewer start?

This is a single file PR, not applicable.

Test plan:

  1. Build the docs from this branch
  2. Load and run the sphinx docs locally
  3. Observe you can pick between the different versions of Triton Server without errors
  • CI Pipeline ID:

21363299

Caveats:

Right now we are using the major/minor/patch format for the version instead of the year/month format. This might be future work depending on the time it takes to develop this for 24.12

Background

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

N/A

@nv-kmcgill53 nv-kmcgill53 added the PR: feat A new feature label Dec 11, 2024
docs/conf.py Dismissed Show dismissed Hide dismissed
docs/conf.py Dismissed Show dismissed Hide dismissed
docs/conf.py Fixed Show fixed Hide fixed
@nv-kmcgill53 nv-kmcgill53 marked this pull request as ready for review December 12, 2024 00:43
@yinggeh
Copy link
Contributor

yinggeh commented Dec 12, 2024

Hi @nv-kmcgill53. In generate_docs.py, there are some regular expression hard coded with main for simplicity back then. Especially the TODO comment documented some issues you might need to address for multi-version view.

Ideally, when users view the docs from an previous version, all the hyperlinks should point to docs/files from the same previous release version.

@nv-kmcgill53
Copy link
Contributor Author

nv-kmcgill53 commented Dec 12, 2024

I think I see what you are saying and I don't believe this applies to this change. The switcher links to the archived docs in the triton archives. This should mean that I don't have to worry about the regex in the current documentation since, when clicking on the switcher, it takes the user to the older docs.

Here is a sample of the data which the switcher is using for the select-ables:

[
    {
        "name": "2.52.0 (current_release)",
        "version": "2.52.0",
        "url": "https://docs.nvidia.com/deeplearning/triton-inference-server/user-guide/docs/index.html"
    },
    {
        "name": "2.51.0",
        "version": "2.51.0",
        "url": "https://docs.nvidia.com/deeplearning/triton-inference-server/archives/triton-inference-server-2510/user-guide/docs"
    },
    {
        "name": "2.50.0",
        "version": "2.50.0",
        "url": "https://docs.nvidia.com/deeplearning/triton-inference-server/archives/triton-inference-server-2500/user-guide/docs"
    },
]

@yinggeh
Copy link
Contributor

yinggeh commented Dec 12, 2024

I think I see what you are saying and I don't believe this applies to this change. The switcher links to the archived docs in the triton archives. This should mean that I don't have to worry about the regex in the current documentation since, when clicking on the switcher, it takes the user to the older docs.

Then it should be fine.

I want to point out that the links in https://docs.nvidia.com/deeplearning/triton-inference-server/archives/ are wrong. For example, Links under "Release 2.49.0" points to 2.48.0.

@nv-kmcgill53
Copy link
Contributor Author

That's odd. I'll talk to the docs team about that

@yinggeh
Copy link
Contributor

yinggeh commented Dec 13, 2024

Is the pipeline ID up-to-date? I downloaded the artifact but nothing shown from the release drop list.
Screenshot 2024-12-12 at 4 10 22 PM

@nv-kmcgill53
Copy link
Contributor Author

@yinggeh You will have to build the docs locally and un-comment this line before building the docs. The reason why you are getting the wrong thing is that the switcher is attempting to get the switcher.json file from the production docs where there is none. The switcher needs to pull from your local file system to populate the correct data.

@nv-kmcgill53 nv-kmcgill53 force-pushed the kmcgill-version-docs branch 2 times, most recently from 80a764c to e7d226d Compare December 13, 2024 19:02
version_short = release
# Theme options are theme-specific and customize the look and feel of a theme
# further. For a list of options available for each theme, see the
# documentation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the link to this documentation?

Copy link
Contributor

@yinggeh yinggeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left comments. LGTM otherwise. Thanks for the contribution!

}

version_short = release
# Theme options are theme-specific and customize the look and feel of a theme
# further. For a list of options available for each theme, see the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# further. For a list of options available for each theme, see the
# further. For a list of options available for each theme, see the

@nv-kmcgill53
Copy link
Contributor Author

Closing this as these changes got picked into this branch: #7807

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feat A new feature
Development

Successfully merging this pull request may close these issues.

2 participants