Add update job operation to support updating name, priority and tags - #764
Add update job operation to support updating name, priority and tags#764v-elegacheva wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟢 Ready to approve
The implementation aligns with the generated client’s merge-patch update API and includes targeted unit tests, with only minor doc/test-surface nits noted.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds an update capability for already-submitted Azure Quantum jobs, enabling callers to change a job’s name, priority, and/or tags via JSON Merge Patch, with supporting public API exposure and tests.
Changes:
- Introduces
Workspace.update_job(...)to patch job name/priority/tags (requiring at least one field). - Adds
Job.update(...)convenience method to update a job and refresh its details in place. - Extends the test mock client and adds unit tests covering success, partial updates, and error cases.
File summaries
| File | Description |
|---|---|
| azure-quantum/tests/test_workspace.py | Adds unit tests validating job update behavior and error handling. |
| azure-quantum/tests/mock_client.py | Adds mock jobs.update(...) implementation to support tests. |
| azure-quantum/azure/quantum/workspace.py | Implements Workspace.update_job(...) using JobUpdateOptions merge-patch. |
| azure-quantum/azure/quantum/job/job.py | Adds Job.update(...) wrapper calling Workspace.update_job(...). |
| azure-quantum/azure/quantum/init.py | Exposes Priority on the public azure.quantum package namespace. |
| .gitignore | Ignores a local scratch/manual test file. |
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
|
||
| :param priority: | ||
| The new priority of the job. | ||
| One of :class:`~azure.quantum.Priority` (``\"Standard\"`` or ``\"High\"``). |
| from unittest import mock | ||
| from azure.quantum.job.job import Job | ||
| from azure.quantum._client.models import JobDetails | ||
| from azure.quantum._client.models import Priority |
Adds Workspace.update_job and Job.update to update a submitted job's name, priority and/or tags via the JobUpdateOptions JSON Merge Patch operation. Only provided fields are sent; requires at least one field. Exposes Priority publicly and adds unit tests plus mock client support.
ef987c7 to
a844063
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
The public API exposure of Priority isn’t actually validated by the new tests, and there’s a docstring escaping issue that will render incorrectly in generated documentation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
azure-quantum/azure/quantum/workspace.py:493
- The docstring escapes quotes inside a literal (
\"Standard\"/\"High\"), which will render the backslashes in generated documentation. Use unescaped quotes inside the reST literal instead.
:param priority:
The new priority of the job.
One of :class:`~azure.quantum.Priority` (``\"Standard\"`` or ``\"High\"``).
azure-quantum/tests/test_workspace.py:12
- The test imports
Priorityfrom the generated_clientpackage, which doesn’t validate the PR’s claim thatPriorityis publicly exposed fromazure.quantum. Importing from the public surface ensures the test covers the intended API contract.
from azure.quantum.job.job import Job
from azure.quantum._client.models import JobDetails
from azure.quantum._client.models import Priority
from azure.quantum._constants import EnvironmentVariables, ConnectionConstants
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
The mock jobs.update() return value does not currently match the generated client’s contract (missing required id), and update_job() should consistently use a single captured job_id to avoid update/get mismatch risk.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
azure-quantum/azure/quantum/workspace.py:526
update_job()usesjob.details.idfor the update call butjob.idfor the subsequent get. If those ever diverge (e.g.,detailsrefreshed/swapped), this can update one job and fetch another. Capture a singlejob_idonce and use it for both calls.
self.subscription_id,
self.resource_group,
self.name,
job.details.id,
update_options)
azure-quantum/tests/mock_client.py:220
JobsOperations.update()returns the sameresourceobject that was passed in, but the real generated client deserializes and returns aJobUpdateOptionsresponse that includes the jobid(required field). Returning an object withoutidcan break callers/tests that rely on the mocked return value matching the service contract.
return resource
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…ateOptions with id from mock
There was a problem hiding this comment.
🟢 Ready to approve
The implementation matches the generated client’s update contract, includes input validation, and is covered by focused unit tests and mock support.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Summary
Adds support for updating a submitted job's name, priority, and/or tags to the Python SDK, implementing the SDK side of the merged Data Plane API Spec. Only the fields you provide are sent, and the updated job is returned. Note:
tagsreplaces existing tags rather than appending.What Changed
Workspace.update_job(job, *, name=None, priority=None, tags=None).Job.update(...)instance method, which updates the job in place.name,priority, ortags); raisesValueErrorotherwise.Priorityenum (Standard/High) publicly.