feat: az provider session image support to proc images - #360
Conversation
There was a problem hiding this comment.
✅ Ready to approve
The functional changes are straightforward and test-covered, with only minor test-file cleanup nits (unused variables) remaining.
Note: this review does not count toward required approvals for merging.
Pull request overview
Adds support in the procurement images CLI for selecting the Azure provider and the VSCodeInteractiveSessions image type, plus extends the procurement image tests to cover these new inputs.
Changes:
- Extend
procurement images set/resetCLI commands to accept providerazureand image typeVSCodeInteractiveSessions. - Update procurement
Imagesmethod docstrings to document Azure and VSCode support. - Add/extend pytest coverage for Azure provider cases, multiple Azure regions, and VSCode session image type.
File summaries
| File | Description |
|---|---|
tests/test_procurement/test_set_procurement_organisation_image.py |
Expands set-image tests to cover Azure provider, Azure regions, and VSCodeInteractiveSessions. |
tests/test_procurement/test_reset_procurement_organisation_image.py |
Expands reset-image tests to cover Azure provider, Azure regions, and VSCodeInteractiveSessions. |
cloudos_cli/procurement/images.py |
Updates method docstrings to include Azure provider and VSCodeInteractiveSessions image type. |
cloudos_cli/procurement/cli.py |
Updates CLI option choices to allow azure provider and VSCodeInteractiveSessions image type for set/reset. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
l-mansouri
left a comment
There was a problem hiding this comment.
@rubengomex we need a changelog entry and the version bump for sure. The rest of the PR seems to be 1 new test and a bunch of "aesthetic" code clean up, is that it?
dapineyro
left a comment
There was a problem hiding this comment.
LGTM! as @l-mansouri says, the only lacking thing is an entry in changelog.md
Ticket: https://lifebit.atlassian.net/browse/LP-112087
TDD: https://lifebit.atlassian.net/wiki/spaces/LPE/pages/2839511063/AA+-+As+client+admin+I+can+provide+my+own+Azure+AMI+versions+for+Lifebit-managed+services+jobs+sessions#CloudOS-cli
AC Proof
List
Set
Reset