Skip to content

Retry on 503 #2246

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
Merged
merged 2 commits into from
Jul 17, 2025
Merged

Retry on 503 #2246

merged 2 commits into from
Jul 17, 2025

Conversation

kt474
Copy link
Member

@kt474 kt474 commented May 2, 2025

Summary

Should add a test

Details and comments

Fixes #2244

@kt474 kt474 marked this pull request as ready for review May 30, 2025 16:36
ElePT
ElePT reviewed Jun 3, 2025
View reviewed changes
Copy link
Collaborator

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

After reviewing the issue, the PR doesn't seem to be addressing:

When there is a maintenance outage we can go through the retry logic first and then show the proper error message at the end.

How can a maintenance outage be detected in this case?

@ElePT ElePT added this to the 0.41.0 milestone Jun 5, 2025
ElePT
ElePT reviewed Jul 16, 2025
View reviewed changes
Copy link
Collaborator

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

I just labeled this PR as new feature but I am not 100% convinced that's the right classification.

@ElePT ElePT added the Changelog: New Feature Include in the Added section of the changelog label Jul 16, 2025
@kt474
Copy link
Member Author

kt474 commented Jul 16, 2025

This PR undos an incorrect removal that was done to fix #2099 - on the new platform I don't think we have a mechanism to display outage messages. My understanding is that we should have this 503 retry to handle cloudfare outages.

The issue of adding back some sort of way to check and display a proper outage message can be a separate issue (which may not be possible at the moment).

ElePT
ElePT approved these changes Jul 17, 2025
View reviewed changes
Copy link
Collaborator

@ElePT ElePT 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.

Alright, in that case we can probably merge this without mentioning it in the changelog. LGTM.

@ElePT ElePT added this pull request to the merge queue Jul 17, 2025
Merged via the queue into Qiskit:main with commit d255d28 Jul 17, 2025
17 checks passed
@kt474 kt474 deleted the retry-on-503 branch July 17, 2025 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ElePT ElePT ElePT approved these changes

Assignees
No one assigned
Labels
Changelog: New Feature Include in the Added section of the changelog
Projects
None yet
Milestone
0.41.0
Development

Successfully merging this pull request may close these issues.

Add retries back on 503s
2 participants
@kt474 @ElePT