Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[Website 2.0] General Version Dropdown #17948

Merged

Conversation

connorgoggins
Copy link
Contributor

@connorgoggins connorgoggins commented Apr 1, 2020

Description

This PR resolves the issues outlined in the Github issue for the MXNet Website 2.0 General Version Dropdown. It serves to add a fully functional API version dropdown with consistent styling to the master website, and also modifies the Jenkins build instructions to download static artifacts from an external repository. Currently, this repository of static artifacts includes websites for the following past versions of the MXNet API:

  • 1.6
  • 1.5.0
  • 1.4.1
  • 1.3.1
  • 1.2.1
  • 1.1.0
  • 1.0.0
  • 0.11.0
  • 0.12.1

Each version of the website also has a similarly formatted API version dropdown in the header of each page, making it easy for users to seamlessly navigate between different versions of the MXNet website.

Checklist

Essentials

  • Changes are complete (i.e. I finished coding on this PR)
  • Code is well-documented
  • To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • M docs/python_docs/themes/mx-theme/mxtheme/header_top.html
  • M docs/python_docs/themes/mx-theme/mxtheme/layout.html
  • M docs/static_site/Makefile
  • M docs/static_site/src/_includes/head.html
  • M docs/static_site/src/_includes/header.html

Comments

Preview is currently available here.

For instructions on adding future release versions (either as static artifacts or via dynamic build pipelines) please read the wiki page or the Confluence page.

@sandeep-krishnamurthy @aaronmarkham @szha @sojiadeshina @leezu

@mxnet-bot
Copy link

Hey @connorgoggins , Thanks for submitting the PR
All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands:

  • To trigger all jobs: @mxnet-bot run ci [all]
  • To trigger specific jobs: @mxnet-bot run ci [job1, job2]

CI supported jobs: [clang, edge, centos-gpu, centos-cpu, windows-cpu, miscellaneous, unix-cpu, windows-gpu, unix-gpu, website, sanity]


Note:
Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin.
All CI tests must pass before the PR can be merged.

@szha
Copy link
Member

szha commented Apr 1, 2020

Thanks for introducing the versioned website.

In terms of appearance, could we make the version selector more visually distinct from other links? From the current look it doesn't seem obvious that a dropdown menu is there.
image
image

Another ask is that since we are maintaining both the mxnet 1.x (v1.x branch, w/ 1.7 to be the next version) and 2.x (master branch, w/ 2.0 to be the next version), can we make nightly builds for these dev branches available? See #17701 for details.

Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

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

I have some organizational preferences for styles... so it is easier to maintain. Can you do that in a followup PR?

@@ -7,6 +7,27 @@
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
<meta http-equiv="x-ua-compatible" content="ie=edge">
<style>
.dropdown {
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 make sure all of the style stuff is documented - so we know what it does and how it interacts with all of the other pieces? Like how does this interact with this?
https://github.com/apache/incubator-mxnet/blob/master/docs/python_docs/_static/mxnet.css

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll create a document/wiki page with styling info once the design is finalized.

@@ -15,4 +15,25 @@

<script src="{{'/assets/js/clipboard.js'|relative_url}}"></script>
<script src="{{'/assets/js/copycode.js'|relative_url}}"></script>
<style>
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicated in layout.html. Should be abstracted to a single source so it's easier to manage.

<div class="dropdown">
<span>master</span>
<div class="dropdown-content">
<a style="color:#FF4500;" href="/">master</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

move to central css

<div class="dropdown">
<span style="color:#FFFFFF">master</span>
<div class="dropdown-content">
<a style="color:#FF4500;font-weight: lighter;" href="/">master</a><br>
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 move styles to central css file?

@connorgoggins
Copy link
Contributor Author

@szha thanks for your feedback! I would be happy to modify the styling of the version dropdown. Can you give me an idea of what exactly you had in mind?

Regarding the nightly build configurations, since v1.x and v2.x use restricted nodes (see Jenkinsfile_website_full) these builds would need to be run on prod Jenkins, which I currently do not have admin access to. At the moment, I am trying to get access to prod Jenkins so I can set up these nightly build pipelines.

@connorgoggins
Copy link
Contributor Author

@szha I created two PRs with changes that will add Jenkinsfiles to support nightly builds for v1.x and v2.x (master). When you have some free time, would you mind reviewing them?

@@ -20,7 +20,7 @@ all: html
html:
mkdir -p build
cd src && bundle install && JEKYLL_ENV=production bundle exec jekyll build --config _config_prod.yml -d ../build/html && cd ..

wget https://mxnet-static-artifacts.s3.amazonaws.com/versions.zip && unzip versions.zip -d build/html && rm -rf build/html/__MACOSX && rm versions.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does that come from? It seems like we're breaking out of a context here. S3 should not be the method how multiple Jenkins jobs exchange data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your question, @marcoabreu. The content for the static artifacts is managed in this repository. Originally this line was a call to directly clone the repository, but based on feedback it was decided that it would be better to not rely on external repositories and pull the artifacts from an S3 bucket instead.

Here, S3 isn't being used as the way multiple Jenkins jobs exchange data per se. There are many older static artifacts in the ZIP that have gone through multiple rounds of changes which are tracked in the aforementioned repository, rather than simply being built by another Jenkins pipeline.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an idea: add this to the Dockerfile so that it gets cached as part of the image and we don't have to grab that zip every time there's a test or build.

Copy link
Contributor

Choose a reason for hiding this comment

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

But how would you invalidate that cache?

I think it's important to have continuity to allow a contributor to understand and also reproduce how our website gets created. As it stands right now, people would not be able to facilitate this.

What happened to the idea with the Jenkins jobs creating artifacts and another job consuming these artifacts to then assemble the website?

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, this is a static artifact that can't be directly updated by the community. For a complete solution, we definitely need them to be generated.

On the other hand, the lack of website versioning is such a pressing issue that it has come up multiple times, even in past release votes, and is still actively causing confusion for many current and prospective users. We must be losing user base because of this.

If the contributors could come up with a plan for a solution that's based on automatically generated content, and execute the plan before any of our next releases, I think we should accept this solution for now to address the immediate need for avoiding confusion.

Copy link
Member

Choose a reason for hiding this comment

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

I see that @connorgoggins is already contributing other PRs in this direction (#17956 #17957), I trust that the contributor has the intention to make the generation automated. @connorgoggins @sandeep-krishnamurthy contingent on if you could spell out a plan for automated generation for the website, I support accepting this patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed @szha. The priority right now should be getting the working version dropdown in this PR into the production site as soon as possible.

@marcoabreu, I have created a confluence page so that all contributors can understand and reproduce the steps to add additional releases to the website. In the future, we will move towards creating dynamic pipelines to automatically generate and integrate new releases of MXNet into the website, but for now I believe the static artifact solution + master integration in this PR serves its purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

@marcoabreu Thanks for your feedback and I fully agree that if generation of web content happens through Jenkins in an automated way, all community members can participate in the updates. In that direction, @connorgoggins already has PRs to make 1.x and 2.x branches auto builds. The fix here is very critical for MXNet community. Without this change, users are unable to use MXNet website. One concern would be if there is a patch release on 1.6.x then someone needs to regenerate static artifacts. Given that @connorgoggins already documented steps for the same and his other contributions I see this as a non-blocking issue that will not leave MXNet community in limbo.
What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me. Thanks

@connorgoggins
Copy link
Contributor Author

Update: the new version of the website preview reflects the changes I have made to this PR to incorporate feedback. @szha the styling of the dropdown in 1.6 and master has been updated to more clearly indicate that the menu option is a dropdown.

@marcoabreu
Copy link
Contributor

To me it's unclear how that magic versions.zip gets created. Could you maybe make a diagram of how the individual versions get created, where they are stored and how they are merged? Some data flow would be good

@connorgoggins
Copy link
Contributor Author

@marcoabreu see "Jenkins Artifact Generation" within the Static Artifact Publication section of the wiki page for detailed instructions on how to add a new artifact, create versions.zip, and upload to S3.

@marcoabreu
Copy link
Contributor

I'm no longer an Amazon employee. This content should be hosted on confluence.

@szha
Copy link
Member

szha commented Apr 2, 2020

The updated style looks good to me. Thanks @connorgoggins

@connorgoggins
Copy link
Contributor Author

@marcoabreu the Confluence page with instructions for adding releases is now public.

Copy link
Contributor

@sandeep-krishnamurthy sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

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

Thanks for your contributions!

@connorgoggins connorgoggins force-pushed the website_version_dropdown branch 3 times, most recently from e046e7c to 398beb5 Compare April 7, 2020 22:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants