-
Notifications
You must be signed in to change notification settings - Fork 128
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
Add retries to DisableCrlValidation test to improve reliability #5537
Conversation
Why is this only relevant in this test? I would expect we'd need to handle this problem in the product as well. How do we know the transport exception is due to the network and not how the request is formulated? |
The issue linked has more info. This is the only test that's failing intermittently due to the CRL validation check timing out for our test site. This test enables that TransportOptions
Ability to deal with transient network failures is handled in the product, via the RetryPolicy. This test is directly calling the transport layer. See the pipeline being used in the tests: azure-sdk-for-cpp/sdk/core/azure-core/test/ut/transport_policy_options.cpp Lines 273 to 276 in 2c83fc6
Because it is intermittent, at 95%+ success rate, and we know the error. The CRL validation check is hitting a transient timeout. If there was an issue with the request, it would be more determinsitic and we'd see different error behavior. |
/azp run cpp - core - ci |
Azure Pipelines successfully started running 1 pipeline(s). |
@danieljurek CI pipeline runs keep getting canceled after 45 minutes. Can you please help with this? Otherwise green runs aren't showing up as passing. |
The win2022 image has been deleted at that version. we'll have to roll forward to the latest image which has the problems detailed here: #5483 ... I'm working on removing unneeded packages based on the mitigation described here: actions/runner-images#9701 |
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.
Can this be merged now?
Fix #5533
It's easiest to review the actual changes in the PR without whitespaces:
https://github.com/Azure/azure-sdk-for-cpp/pull/5537/files?diff=split&w=1