-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Updates to azure autoscaler for authentication and dependency updates #19603
Conversation
…oving to azure-identity based authentication
Thanks for the fix! |
@richardliaw what is needed to update the ci runs to include the azure sdk packages added here? |
@ijrsvt @DmitriGekhtman could you help shepherd this PR? |
@gramhagen If you update https://github.com/ray-project/ray/blob/master/python/requirements.txt, then CI will run with the correct versions! |
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.
Overall looks good--just two comments :)
…e sdk function resolution
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.
One minor comment, but otherwise LGTM!
Can this PR be merged now && will it be in the next Ray release? Thanks! |
@gramhagen Can you rebase this on master? I'm unsure why CI is not wanting to build for this |
bumping this, what needs to happen to get this merged? @ijrsvt |
@gramhagen Sorry! Can you rebase against master? |
Bump |
@ijrsvt Ian, I'm a bit lost on how to resolve these tests, any suggestions? |
Hmm @gramhagen, I'm not 100% sure what the best way is to reproduce this, but it looks like some timeout with pip installs? |
merging again, hopefully that was due to aws outage? |
Hmm, I tried to make the hanging command print more output here: https://buildkite.com/organizations/ray-project/pipelines/ray-builders-pr/builds/20646/jobs/a6d7f7e9-d580-4fd4-bab7-3952a8666dfd/raw_log It appears to be hanging on finding a version for |
i expect pyasn1-modules is just the straw that broke the camels back, once pip starts trying to resolve different dependency versions it's super slow, and it looked like there were a few instances of that happening. pinning the dependencies is probably the right way to go, but it will be a bit tedious to find them all. is this replicable in a docker env? |
@gramhagen I'm not 100% sure of the exact docker env setup, but using these requirements.txt and Some possible resolutions:
|
Alright @gramhagen Sorry for the confusion and delays here, but after various attempts at pinning dependencies (#20940 & #21105), it looks like just updating pip was the solution #21111 I can't seem to push to your branch, but I ran tests with merging master here. **If you can merge master again, this should fly through CI =) 🤞 ** |
great news, thanks @ijrsvt ! |
Why are these changes needed?
Related issue number
Closes #19523
Fixes #15274
Checks
scripts/format.sh
to lint the changes in this PR.