-
Notifications
You must be signed in to change notification settings - Fork 134
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 job APIs to armada client #3623
Conversation
Signed-off-by: Jason Parraga <sovietaced@gmail.com>
Signed-off-by: Jason Parraga <sovietaced@gmail.com>
Signed-off-by: Jason Parraga <sovietaced@gmail.com>
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 LGTM, but there's Python-related checks failing.
We should probably expose the other two endpoints as well: |
I can do that here. Will ping when I'm done. |
Any airflow operator related build is going to fail on master right now because the build runs on grpc-io 1.64.0 and anything > than grpc-io 1.62.0 ish pulls in python-protobuf 5.X.X, which is strictly incompatible with apache airflow (through a transitive dependency). I'm working to fix that right now here: open-telemetry/opentelemetry-python#3931 |
Signed-off-by: Jason Parraga <sovietaced@gmail.com>
Signed-off-by: Jason Parraga <sovietaced@gmail.com>
@ClifHouck should be good for review |
I'd hold off on this for now as we'll be syncing out our improved Armada Airflow Operator, which as you say makes use of the query API as opposed to JobService. We'll get these exact changes as part of that work, which should be done in about a week. |
Closing, see #3650 |
This pull request adds a
get_job_status
method to the armada python client. This is something that we'd like to use in order to migrate to the new query API. I figure the armada airflow operator will probably need to use this sooner or later as well as it migrates to the new query API.