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.

Sign up for GitHub

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

Jump to bottom

tools: add NODE_TEST_NO_INTERNET to the doc builder #31849

Closed
wants to merge 1 commit into from
Closed

tools: add NODE_TEST_NO_INTERNET to the doc builder #31849

wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

Fixes: #29918

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@joyeecheung
tools: add NODE_TEST_NO_INTERNET to the doc builder
23db669 
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Feb 18, 2020
@joyeecheung
Copy link
Member Author

cc @nodejs/documentation

@mmarchini
Copy link
Contributor

Shouldn't it fail if NODE_TEST_NO_INTERNET is set on a release build? It seems like the environment variable allows users to bypass that check now

@joyeecheung
Copy link
Member Author

joyeecheung commented Feb 18, 2020 •
edited
Loading

@mmarchini I don't think it's harmful for a release build to bypass that check when the environment variable is set anyway? Our release machines certainly don't have it set and at worst the links in the docs could get away with being a 404 which is not really the end of the world. Better still, it allows people to build releases when they are behind a firewall and explicitly opt out of these checks (which are probably not useful to them anyway).

richardlau
richardlau approved these changes Feb 18, 2020
View reviewed changes
Copy link
Member

@richardlau richardlau left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think this is a reasonable stop gap solution. Ideally we don't need to attempt to access the internet for every doc built -- We could work out and store to disk the alternate versions ahead of building the docs and then read them in during the build but that can be done in a follow up PR (I up for doing it if I can carve out some time).

@joyeecheung joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 19, 2020
joyeecheung added a commit that referenced this pull request Feb 25, 2020
@joyeecheung
tools: add NODE_TEST_NO_INTERNET to the doc builder
914d800 
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@joyeecheung
Copy link
Member Author

Landed in 914d800, thanks!

codebytere pushed a commit that referenced this pull request Feb 27, 2020
@joyeecheung @codebytere
tools: add NODE_TEST_NO_INTERNET to the doc builder
f293dcf 
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@codebytere codebytere mentioned this pull request Feb 29, 2020
Merged
@richardlau richardlau mentioned this pull request Mar 4, 2020
Merged
3 tasks
codebytere pushed a commit that referenced this pull request Mar 15, 2020
@joyeecheung @codebytere
tools: add NODE_TEST_NO_INTERNET to the doc builder
7840f46 
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
@joyeecheung @codebytere
tools: add NODE_TEST_NO_INTERNET to the doc builder
e635b53 
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@codebytere codebytere mentioned this pull request Mar 17, 2020
Merged
@richardlau richardlau mentioned this pull request Mar 27, 2020
Closed
codebytere pushed a commit that referenced this pull request Mar 30, 2020
@joyeecheung @codebytere
tools: add NODE_TEST_NO_INTERNET to the doc builder
1060a2b 
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@richardlau richardlau mentioned this pull request Apr 3, 2020
Closed
3 tasks
richardlau pushed a commit to richardlau/node-1 that referenced this pull request Apr 6, 2020
@joyeecheung @richardlau
tools: add NODE_TEST_NO_INTERNET to the doc builder
2c3c6a0 
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

Backport-PR-URL: nodejs#32642
PR-URL: nodejs#31849
Fixes: nodejs#29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 6, 2020
@joyeecheung @BethGriggs
tools: add NODE_TEST_NO_INTERNET to the doc builder
3756be8 
At the moment the doc builder tries to access the internet
for CHANGELOG information and only falls back to local sources
after the connection fails or a 5 second timeout. This means
that the doc building could take at least 7 minutes on a
machine with hijacked connection to Github for useless network
attempts. This patch adds a NODE_TEST_NO_INTERNET environment
variable to directly bypass these attempts so that docs can be
built in reasonable time on a machine like that.

Backport-PR-URL: #32642
PR-URL: #31849
Fixes: #29918
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Apr 7, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

@mmarchini mmarchini mmarchini approved these changes

@richardlau richardlau richardlau approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test/doctool/test-doctool-html requires internet connection?
7 participants
@joyeecheung @mmarchini @lpinca @cjihrig @richardlau @BridgeAR @nodejs-github-bot