-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(spark): [O11YINFRA-46] retry connection error to account for auto… #21922
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
base: master
Are you sure you want to change the base?
Conversation
3b0035d to
23c74f3
Compare
cc833ad to
05777a6
Compare
0d68ee5 to
a1cfa25
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files🚀 New features to boost your workflow:
|
019d77c to
90f535c
Compare
…discovery race condition
90f535c to
26ae584
Compare
| if pod_phase is None: | ||
| return False | ||
|
|
||
| if pod_phase in ('failed', 'succeeded', 'unknown'): |
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.
Do you know what happens if the pod is pending? Does autodiscovery pick up config from pending pods?
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.
Great question - I have not seen an example of this, but I do not know for sure. I could do a bit of research to try to see.
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.
It's more a of a nit more than anything feel free to timebox it and convert this into a PR.
…discovery race condition
What does this PR do?
Motivation
Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged