Skip to content
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

Status api versions #29

Merged
merged 2 commits into from
Jun 19, 2024
Merged

Status api versions #29

merged 2 commits into from
Jun 19, 2024

Conversation

Iain-S
Copy link
Contributor

@Iain-S Iain-S commented May 30, 2024

Summary

For the newer API versions (e.g. the one we're dealing with as of the recent dependency updates), we need to call auth_client.role_assignments.list_for_subscription() instead of auth_client.role_assignments.list().

Detail

The azure.mgmt.authorization import AuthorizationManagementClient decides dynamically what version of role_assignments it will return based on the api_version passed to the constructor or else a default. This makes it hard for type-checkers to pick up on the change.

This PR:

  1. Explicitly requires version 2022-04-01. There is nothing special about it other than it seems to be the latest stable version.
  2. Dynamically loads the right versions of RoleAssignment and RoleAssignmentOperations for use in testing either:
    • directly, as in MODELS_MODULE.RoleAssignment()
    • as a spec for Mock, as in MagicMock(spec=OPERATIONS_MODULE.RoleAssignmentsOperations)

@Iain-S Iain-S force-pushed the status-api-versions branch from ee7572c to 3993643 Compare May 30, 2024 15:55
@Iain-S Iain-S requested a review from dlpbc May 30, 2024 15:55
@Iain-S Iain-S added the WIP Work In Progress (don't merge) label May 30, 2024
@Iain-S Iain-S force-pushed the status-api-versions branch from 3993643 to a58be53 Compare May 30, 2024 16:16
@Iain-S Iain-S removed the WIP Work In Progress (don't merge) label May 30, 2024
@Iain-S Iain-S force-pushed the status-api-versions branch from a58be53 to 36ad1c4 Compare May 30, 2024 16:18
Copy link
Contributor

@dlpbc dlpbc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
The comments about the PR were also helpful to understand the update. Thanks!

@Iain-S Iain-S merged commit 84b9bb5 into main Jun 19, 2024
7 checks passed
@Iain-S Iain-S deleted the status-api-versions branch June 19, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants