-
Notifications
You must be signed in to change notification settings - Fork 24
fix: grab the first value if the server response contains a comma #482
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
Conversation
baywet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
Can you please add a unit test for that scenario to prevent future regressions?
|
@microsoft-github-policy-service agree |
|
Hi @baywet, I have added a test, I hope it is conforming to what you were expecting, let me know if I need to change something :)! |
baywet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for making the changes!
|
@MarcoP9 can you have a look at the failure for the http unit tests please? https://github.com/microsoft/kiota-python/actions/runs/15881670608/job/44846104287?pr=482 Disregard the failing serialization CI. I've authored #483 to address this. |
|
Hi @baywet, I have fixed the unit test that was failing. My bad, I did not consider that other http dates might contain commas. I have now made the check more specific, the first value is grabbed only if a regex pattern is matched: Let me know if it sounds good! |
|
@MarcoP9 this looks like a narrow implementation. What of we don't exactly have the same number of digits? |
|
@baywet I thought about that too, however, the server is returning a retry-after value
What do you think? |
|
what if we kept the code as is, but relaxed the regex a little instead? |
|



Overview
After encountering a 429 Too Many Requests Error, the server returns an incorrect Retry-After header of "30,120", resulting in an exception in the retry handler. As suggested by @musale in the comments, the function is now splitting the string by the comma and returning the first value.
Related Issue
Contributes to fix: microsoftgraph/msgraph-sdk-python#1159