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

[DOC] fix installation selector wrong history #16381

Merged
merged 3 commits into from
Oct 10, 2019
Merged

Conversation

szha
Copy link
Member

@szha szha commented Oct 6, 2019

Description

fix installation selector wrong history. it current hardcodes to the index page.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)

Changes

  • fix installation selector wrong history. it current hardcodes to the index page.
  • remove repetitive code in the option selector.

@szha
Copy link
Member Author

szha commented Oct 6, 2019

@aaronmarkham I can't find the s3 preview link in the PR check anymore. Where can I find it?

@marcoabreu
Copy link
Contributor

Any particular reason you're removing the history push?

@ThomasDelteil
Copy link
Contributor

@szha the s3 PR preview has been disabled for now, links have been prettified which means we need an apache web server to render the website properly since we rely on .htaccess rewrite rules to expand URLs.
The solution we discussed with @aaronmarkham is to deploy all PR websites on dev box with a large disk and serve them under an httpd endpoint in the form of https://dev-website.apache.com/PR-12345/5/index.html

@ThomasDelteil
Copy link
Contributor

Thanks for the PR @szha, I see what happened there.
The URL used to be https://mxnet.apache.org/get_started/index.html, the selector stopped working once we started using /get_started.

Copy link
Contributor

@ThomasDelteil ThomasDelteil left a comment

Choose a reason for hiding this comment

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

I know until we get back the PR preview back it's not simple to test the changes, apologies for that, the simplest way currently to test the front end is to build the website using:
ci/build.py --docker-registry mxnetci --platform ubuntu_cpu_jekyll /work/runtime_functions.sh build_jekyll_docs

@szha
Copy link
Member Author

szha commented Oct 6, 2019

@ThomasDelteil thanks for the review. I will address them soon.

@marcoabreu I'm not removing the history push. I refactored that part of the code by moving the repetition out of the conditional clauses.

@marcoabreu
Copy link
Contributor

Oh yeah now I see it. Sorry about that

@szha
Copy link
Member Author

szha commented Oct 7, 2019

we rely on .htaccess rewrite rules to expand URLs

Are there some examples? Could we use s3 for that? Hosting through a webserver will increase operational cost.

@ThomasDelteil
Copy link
Contributor

@szha, on the cost front: I think a smartly hosted webserver with symlinks for same files across websites vs 200MB for every commits on every PR on s3 will have a comparable operational cost. I don't think we need a very powerful machine, a C4 ~40$/month should be plenty. Which is a drop compared to the CI cost itself.

Examples are like this:
https://mxnet.apache.org/api => https://mxnet.apache.org/api/index.html

@ThomasDelteil
Copy link
Contributor

@szha committed the updates, tested here: http://54.211.76.74/get_started

@szha szha merged commit 6ce323f into apache:master Oct 10, 2019
@szha szha deleted the fix_install branch October 10, 2019 22:52
aaronmarkham pushed a commit to aaronmarkham/incubator-mxnet that referenced this pull request Oct 16, 2019
* fix installation selector wrong history

* updates to script

* nudge CI
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.

3 participants