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

WIP: urlchecker github action #591

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

shahzebsiddiqui
Copy link
Contributor

No description provided.

@shahzebsiddiqui
Copy link
Contributor Author

@boegel this should work once approved and added to easybuilders project from marketplace https://github.com/marketplace/actions/urls-checker

Invoke action on develop,master upon PR and only on push to master
branch.
Since most documentation will get merged to develop first, it makes no
sense to check a PR to master branch. Its only in case if push is
directed to master branch directly without going through develop first.
@ocaisa ocaisa closed this Feb 7, 2020
@ocaisa ocaisa reopened this Feb 7, 2020
@ocaisa
Copy link
Member

ocaisa commented Feb 7, 2020

Sorry I didn't mean to do that!

@ocaisa
Copy link
Member

ocaisa commented Feb 7, 2020

IMO this is way too slow, we need to be able to whitelist all of our repo links and only check external links. From the docs, it doesn't look like white-listing https://github.com/easybuilders/easybuild-easyconfigs/* is possible

@ocaisa
Copy link
Member

ocaisa commented Feb 7, 2020

I tell a lie, it is possible...

@ocaisa
Copy link
Member

ocaisa commented Feb 7, 2020

Note that this clones the repo and doesn't take a branch as an option, so can only do something useful on a push to master.

Also, it can't deal with our templating (%(version)s) in the URLs, so there will always be manual work to keep a white-list up to date.

@shahzebsiddiqui shahzebsiddiqui changed the title urlchecker github action WIP: urlchecker github action Feb 7, 2020
@boegel boegel added the change label Feb 8, 2020
@boegel boegel added this to the 4.x milestone Feb 8, 2020
@ocaisa ocaisa mentioned this pull request Feb 8, 2020
@ocaisa
Copy link
Member

ocaisa commented Feb 10, 2020

FWIW Our templating is a real problem with this and the urlchecker crashes with some of our URLs:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/requests/models.py", line 379, in prepare_url
    scheme, auth, host, port, path, query, fragment = parse_url(url)
  File "/usr/local/lib/python3.8/site-packages/urllib3/util/url.py", line 392, in parse_url
    return six.raise_from(LocationParseError(source_url), None)
  File "<string>", line 3, in raise_from
urllib3.exceptions.LocationParseError: Failed to parse: http://%(namelower)s.googlecode.com/files

@shahzebsiddiqui
Copy link
Contributor Author

@ocaisa i have submitted a ticket to developer for feedback urlstechie/urlchecker-action#7.

@shahzebsiddiqui
Copy link
Contributor Author

@ocaisa even if this URL-checker may not be merged due to some of the issues. I think going through the report of all invalid URLs and fixing them first would be one major improvement. For end-user reading documentation and coming through a broken link can be problematic.

@shahzebsiddiqui
Copy link
Contributor Author

@boegel and @ocaisa the github action has changed its now https://github.com/urlstechie/urlchecker-action which is reimplementation of the one I sent you.

I think you can setup filter on what files to check or exclude. Anyhow I am happy to help resolve this old PR by refactoring this or we can close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants