Stop extensively test no longer used gui.dandiarchive.org, retry HEAD (redirects) upon requests.ConnectionError#1632
Conversation
We long migrated to just dandiarchive.org . Excessive use of gui. resulted in too many redirects and seems like likely heroku limiting our requests thus tests some times failing to get all the way to redirected site. But since we no longer even use gui. domain, there is no need to test it. I also adjusted test to exclude /# from URL since also no longer there and would be redirected/handled.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1632 +/- ##
==========================================
+ Coverage 88.68% 88.84% +0.16%
==========================================
Files 82 82
Lines 11400 11434 +34
==========================================
+ Hits 10110 10159 +49
+ Misses 1290 1275 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
One Ubuntu still failed that test_follow_redirect |
|
changes to |
We keep getting ConnectionResetErrors on windows while following redirects. That was not handled at all, so we did not retry. Added support for retries there Closes #1604
yarikoptic
changed the title
Extends existing test_follow_redirect with two new test cases: - test_follow_redirect_retry_on_connection_error: verifies successful retry after ConnectionError - test_follow_redirect_exhausted_retries_on_connection_error: verifies proper exception handling after retry exhaustion These tests complement the retry functionality added in b5dbeb9 to handle frequent ConnectionErrors on Windows during redirect following. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
As green as it gets now. Reviewers invited (attn @candleindark and @asmacdo ) |
asmacdo
left a comment
asmacdo
left a comment
There was a problem hiding this comment.
Duplicate params should be removed, other suggestions are just nitpicks and can be ignored.
otherwise we get typing check failing us. Was not present when we had "while True" loop before
|
ha -- that typing check complaint now might have likely be a sideeffect of replacing that |
|
very green, very good. Let's proceed |
|
🚀 PR was released in |
2 participants
We long migrated to just dandiarchive.org . Excessive use of gui. resulted in too many redirects and seems like likely heroku limiting our requests thus tests some times failing to get all the way to redirected site. But since we no longer even use gui. domain, there is no need to test it. I also adjusted test to exclude /# from URL since also no longer there and would be redirected/handled.
I would consider it
Also added one more change to test redirect actually against this gui. (but once) instead of bit.ly -- hopefully would be more robust on windows.