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

scripts: link check improvements #1000

Closed
casperdcl opened this issue Feb 15, 2020 · 10 comments · Fixed by #1619
Closed

scripts: link check improvements #1000

casperdcl opened this issue Feb 15, 2020 · 10 comments · Fixed by #1619
Assignees
Labels
A: website Area: website p0-critical Affects users in a bad way at the moment type: discussion Requires active participation to reach a conclusion. type: enhancement Something is not clear, small updates, improvement suggestions

Comments

@casperdcl
Copy link
Contributor

casperdcl commented Feb 15, 2020

UPDATE (by @shcheklein) - making it p0 since people stopped paying attention to CI due to the last item in the list below. We can make the last item only as a separate p0 PR.


Regarding checking for dead links:

@casperdcl casperdcl added type: discussion Requires active participation to reach a conclusion. type: enhancement Something is not clear, small updates, improvement suggestions labels Feb 15, 2020
@casperdcl casperdcl self-assigned this Feb 15, 2020
@casperdcl
Copy link
Contributor Author

btw @shcheklein if you're happy to revert

(find pages/ public/static/docs/ src/ .github/ -name '*.md' -o -name '*.js' && ls *.md *.js) \
back to using git instead of find then it would be much easier to actually use git's internal .gitignore logic to exclude files

@jorgeorpinel jorgeorpinel changed the title test: links improvements scripts: link check improvements Feb 19, 2020
@casperdcl casperdcl removed their assignment Mar 18, 2020
@jorgeorpinel

This comment has been minimized.

@shcheklein
Copy link
Member

Updated. Made the last item in the list p0, since people stopped paying attention to CI.

@rogermparent
Copy link
Contributor

rogermparent commented Jul 21, 2020

The first step to solve this is to set the predictable Heroku deploy URLs on the dvc.org Heroku project. This opens up the question of which prefix to use, but I would think the example behavior of just using the project name, dvc-landing, is pretty acceptable to everyone since nothing but machines will interact with these URLs.

From there, we'll probably have to make sure the link checker is able to handle the case where a CI check runs before the deploy preview is up. The current integrations on Heroku or CircleCI may already handle this, but if so I'm unaware.

Looking at the link check scripts, it should be pretty easy to adapt them to the predictable Heroku PRs once Heroku and CircleCI are set up for it. There is a quirk in the CircleCI builtin env vars CIRCLE_PR_NUMBER env var saying it only is present on "forked PRs", which could be interpreted to mean it isn't friendly to branch workflow but I'm not sure- especially in the present day.

Though, if that is the case, I believe we can chop up the CIRCLE_PULL_REQUEST env var which is present on every run and get the PR number from that. It just seems really weird if we'd have to do that, but the option exists in case the worst case is true.

I can make the Heroku changes and get started at any time, but I think it'd prudent to hold off until I get the go-ahead in case there's a particular reason we don't already have predictable preview URLs. Would @shcheklein or @jorgeorpinel know if this is the case?

@shcheklein
Copy link
Member

@rogermparent consider this option (if it's technically possible):

  • use Github action for this check
  • in Github action hopefully there is a way to run it when "environment" is ready and keep it pending until it is deployed
  • hopefully we should be able to get the environment URL from the GH action

Also, for the forked PRs- Heroku doesn't deploy them automatically, means that we can't keep CircleCI running infinitely .

@casperdcl
Copy link
Contributor Author

casperdcl commented Jul 22, 2020

handle the case where a CI check runs before the deploy preview is up

hopefully there is a way to run it when "environment" is ready and keep it pending until it is deployed

What's wrong with periodically pinging the predictable url with a timeout?

@shcheklein
Copy link
Member

What's wrong with periodically pinging the predictable url with a timeout?

wasting resources? deploy might take >10mins

@jorgeorpinel
Copy link
Contributor

Quick comment: we're still getting 404 errors when new docs are added in a PR e.g. https://app.circleci.com/pipelines/github/iterative/dvc.org/5841/workflows/3fb81bb8-eb90-42f7-8bf7-6e845b278ecb/jobs/5897 (for #1705). Do we have a separate issue or hope to be able to fix that at some point? 🙂

Thanks

@rogermparent
Copy link
Contributor

@jorgeorpinel Try merging master into your branch, the GitHub Actions-based checker that fixes the issue runs off the workflow files in the PR, so PRs made before the new checker was added will need to be merged before the issue is solved in a particular PR.

@jorgeorpinel
Copy link
Contributor

Ah makes sense, OK cool np then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: website Area: website p0-critical Affects users in a bad way at the moment type: discussion Requires active participation to reach a conclusion. type: enhancement Something is not clear, small updates, improvement suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants