[ACR] Fixing credential_scopes kwarg #19664
Conversation
|
/azp run python - containerregistry - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - containerregistry - tests-weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - containerregistry - tests-weekly |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -139,6 +169,9 @@ def test_update_repository_properties(self, containerregistry_anonregistry_endpo | |||
|
|
|||
| @acr_preparer() | |||
| def test_update_tag_properties(self, containerregistry_anonregistry_endpoint): | |||
| if not self.is_public_endpoint(containerregistry_anonregistry_endpoint): | |||
There was a problem hiding this comment.
I think you can do a pytest.mark.skipif instead, even though you're looking at a self parameter
There was a problem hiding this comment.
I get a NameError: name 'self' is not defined if I do this, also the parameter is passed in by the preparer so it wouldn't be available either.
| @@ -43,13 +43,12 @@ def __init__(self, endpoint, credential, **kwargs): | |||
| if not endpoint.startswith("https://") and not endpoint.startswith("http://"): | |||
| endpoint = "https://" + endpoint | |||
| self._endpoint = endpoint | |||
| self.credential_scope = kwargs.get("authentication_scope", "https://management.core.windows.net/.default") | |||
| self.credential_scopes = kwargs.get("credential_scopes", ["https://management.core.windows.net/.default"]) | |||
There was a problem hiding this comment.
should this be a private attr?
There was a problem hiding this comment.
It probably should, but the ExchangeClient models are not exposed publicly
| @@ -74,7 +73,7 @@ def get_refresh_token(self, service, **kwargs): | |||
| def exchange_aad_token_for_refresh_token(self, service=None, **kwargs): | |||
| # type: (str, Dict[str, Any]) -> str | |||
| refresh_token = self._client.authentication.exchange_aad_access_token_for_acr_refresh_token( | |||
| service=service, access_token=self._credential.get_token(self.credential_scope).token, **kwargs | |||
| service=service, access_token=self._credential.get_token(*self.credential_scopes).token, **kwargs | |||
There was a problem hiding this comment.
Is credential_scopes an iterable (the name implies that it is)? If so, does the keyword paramtype need to be updated from str to something like List[str]?
There was a problem hiding this comment.
Yes, it should be a List[str].
* renaming kwarg and passing through * fixing pylint issue * removing unnecessary cred scopes kwarg pops * adding tests for sovereign cloud * fixing kwarg * fixing invalid cred scope test * mccoys comments
No description provided.