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

not fallback to get request on 405 only #79

Merged
merged 1 commit into from
Nov 8, 2024
Merged

Conversation

john0isaac
Copy link
Owner

Q A
Bug fix? yes
Related issues? Azure-Samples/azure-search-openai-demo#2124

What's in this Pull Request?

We will attempt to send a head request then get request on all urls until a request returns 200 or the retries are 0

We will attempt to send a head request then get request on all urls until a request returns 200 or the retries are 0
@john0isaac
Copy link
Owner Author

@pamelafox tech community was returning 400 not 405 when you send a head request that's why the get request was never executed.
I fixed this but it will make the workflow take more time to execute as it will try head and get 3 times for 10 seconds on each url before flagging it as broken (only if the url is not working) if your urls are working it shouldn't change the execution time of the workflow.

@john0isaac
Copy link
Owner Author

Right now I'm thinking of adding a fail-fast cli flag as it could take forever to finish if you have many broken urls 😆

@john0isaac john0isaac merged commit 7d21152 into dev Nov 8, 2024
12 of 14 checks passed
@john0isaac john0isaac deleted the fix-urls-issue branch November 8, 2024 19:33
@pamelafox
Copy link

Agreed that this can affect potential execution time. I'll keep an eye out for that. Thanks for making the fix! I assume we need to upgrade the version in the workflow for azure-search-openai-demo?

@john0isaac
Copy link
Owner Author

Nope, the workflow always uses the latest version of the python package.
No more changes are required it will work right away.

@pamelafox
Copy link

Ah, great, thanks so much!

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

Successfully merging this pull request may close these issues.

2 participants