search

urlstechie / urlchecker-action

:octocat: :link: GitHub action to extract and check urls in code and documentations.
https://urlchecker-python.readthedocs.io
MIT License
34 stars 12 forks source link

Retry if failed parameters? #15

Closed vsoch closed 4 years ago

vsoch commented 4 years ago

hey @SuperKogito ! We are using your action for buildtest, and I'd like to also test it for usrse https://github.com/USRSE/usrse.github.io/issues/171 where we've been using html-proofer. The issue with proofer is that it doesn't have any understanding / implementation for a retry - many of the links in our static site are old HPC documentation or other servers that don't always respond reliably, and might need a retry with exponential backoff up to some number of maximum attempts. Is this something we could look into adding here? I could definitely open a PR if you want to discuss how to go about it (and expose variables to the user).

SuperKogito commented 4 years ago

Hello @vsoch, I am glad you are interested in using URLs-checker :smiley: .

Well a retry on fail is possible to implement by editing urlproc.py#L46. The edit should consist of adding a while loop that a termination using a user defined input variable number_of_retrials and a success response_status. Such edit should help with your use case, but I think a more global/broad solution is better since the action fails when testing some correct links for some reason, see #4 and #8. A better solution imo is to combine that retry while loop with reporting on the returned error type. So if requests-lib is used appropriately, I think we can also return the type of error and even filter some errors out or ignore them. We can also add the timeout variable in there to the list of input variables by the user to set the amount of time the checker will wait before giving up on the link.

A PR is always welcome, I can use all the help I get and I always welcome suggestions and discussions of improvements, so please feel free to do it :smiley:

vsoch commented 4 years ago

Great! I can take a first shot, hopefully something this week.

vsoch commented 4 years ago

This is merged into master, so closing issue. We can open a separate issue if something needs fixing or update.

SuperKogito commented 4 years ago

The urls were not all printed during the checks. This was due to the initialization of do_retry and retries_count being outside the urls loop

SuperKogito commented 4 years ago

This should be solved in #21

vsoch commented 4 years ago

Ack, thanks for fixing it up! The repo I tested on didn't have any retries so I missed it.

SuperKogito commented 4 years ago

no worries, hopefully it is bug-free now, I will release it and go to sleep. Feel free to run some tests around and let me know how it goes ;)

vsoch commented 4 years ago

Sounds good! I will likely be able to test it fully with actual projects for other orgs once we get the branch fix in - most testing set ups are testing a non standard (non master) branch so they'd want to just use actions/checkout@v2. More tomorrow! Good night!

vsoch commented 4 years ago

okay I've opened #24 - it was tested on buildtest and adds what we discussed plus some other fixes that I ran into while testing. The test is timing out so I'll need your help to take a look!