[Container Registry] Update API for Beta 2 by seankane-msft · Pull Request #18392 · Azure/azure-sdk-for-python

seankane-msft · 2021-04-28T18:10:15Z

No description provided.

...tainerregistry/azure-containerregistry/azure/containerregistry/_container_registry_client.py

sdk/containerregistry/azure-containerregistry/azure/containerregistry/_container_repository.py

annatisch · 2021-05-05T20:34:09Z

sdk/containerregistry/azure-containerregistry/azure/containerregistry/_container_repository.py

+    from ._models import ContentProperties
+
+
+class ContainerRepository(ContainerRegistryBaseClient):


Why is this no longer a client? Is it because a user cannot instantiate it themselves?

A user could instantiate it themselves but it would look a little awkward. We received feedback from the service team that the two clients made it confusing on where a user should start. Krystof and Anne Thompson came up with a design where both ContainerRepository and RegistryArtifact have client-like properties (being able to make service calls) but are more like models. This produces a more intuitive starting point with ContainerRegistryClient.

annatisch · 2021-05-05T20:38:13Z

sdk/containerregistry/azure-containerregistry/azure/containerregistry/_models.py



-class RegistryArtifactOrderBy(str, Enum):
+class ManifestOrderBy(str, Enum):


I feel like this should just be called ManifestOrder or ManifestOrdering.
Then it reads better: order_by=ManifestOrder.LAST_UPDATE_TIME_DESCENDING

annatisch · 2021-05-05T20:39:38Z

sdk/containerregistry/azure-containerregistry/azure/containerregistry/_registry_artifact.py

-        # type: (str, str, TokenCredential, Dict[str, Any]) -> None
-        """Create a ContainerRepositoryClient from an endpoint, repository name, and credential
+class RegistryArtifact(ContainerRegistryBaseClient):
+    def __init__(self, endpoint, repository, tag_or_digest, credential, **kwargs):


It looks like this is also kind of a client - but it didn't have the pipeline/transport wrapping in the methods that construct it.

The reasoning is the same as for ContainerRegistry, but I do need to add the pipeline/transport wrapping constructs.

…into acr-container-repo

annatisch · 2021-05-07T19:36:02Z

sdk/containerregistry/azure-containerregistry/azure/containerregistry/_container_repository.py

+
+    @distributed_trace
+    def list_manifests(self, **kwargs):
+        # type: (Dict[str, Any]) -> ItemPaged[ArtifactManifestProperties]


Are the listed items here the same data that I would get from a get_manifest_properties call?
We're using the same response object for both - so just checking that they both include the same information.

Yes they are, both calls return the same data.

annatisch · 2021-05-07T19:39:38Z

...inerregistry/azure-containerregistry/azure/containerregistry/aio/_async_registry_artifact.py

        :paramtype last: str
        :keyword order_by: Query parameter for ordering by time ascending or descending
-        :paramtype order_by: :class:`~azure.containerregistry.TagOrderBy`
+        :paramtype order_by: :class:`~azure.containerregistry.TagOrder`


The param documentation should include str as a valid input where ever we also use enums :)
Though it doesn't need to be fixed for this preview.

annatisch · 2021-05-07T19:44:53Z

sdk/containerregistry/azure-containerregistry/azure/containerregistry/__init__.py

+    "ContainerRepository",
    "ContentProperties",
-    "ManifestOrderBy",
+    "DeleteRepositoryResult",


What information do we get back from a delete operation?

The manifests and tags deleted as part of the repository delete operation

annatisch · 2021-05-07T19:46:03Z

sdk/containerregistry/azure-containerregistry/azure/containerregistry/_models.py

    :vartype last_updated_on: :class:`datetime.datetime`
    :ivar int manifest_count: Number of manifest in the repository
    :ivar str name: Name of the repository
-    :ivar str registry: Registry the repository belongs to


We don't think this would be useful info?

This was an error on my part, the registry is not included in the response object.

annatisch · 2021-05-07T19:53:11Z

sdk/containerregistry/azure-containerregistry/azure/containerregistry/_container_repository.py

+        if not endpoint.startswith("https://") and not endpoint.startswith("http://"):
+            endpoint = "https://" + endpoint
+        self._endpoint = endpoint
+        self.name = name


Should this also have something like the property on RegistryArtifact:

self.fully_qualified_name = self._endpoint + self.repository

Yes adding it now.

ghost · 2021-05-07T20:02:49Z

Hello @seankane-msft!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

…into azure_purview_scanning * 'master' of https://github.com/Azure/azure-sdk-for-python: (31 commits) [purview] add catalog client (Azure#17788) Add spellcheck for modified files (Azure#18496) [Container Registry] Update API for Beta 2 (Azure#18392) First version of Confidential Ledger Python SDK (Azure#17951) [llc] add quickstart (Azure#18537) [Storage][Fix]Copy source is redirecting (Azure#18577) [Tables] pre-release script (Azure#18505) Update changelogs (Azure#18575) [formrecognizer] Adding to_dict() on custom models (Azure#18402) Raise CredentialUnavailableError when CLI subprocess times out (Azure#18509) Extend Check Enforcer timeout (Azure#18526) Core raw streaming (Azure#17920) hide secrets in mgmt sdk (Azure#18535) add filter samples for list methods (Azure#18480) Revert changes to SetDevVersion. (Azure#18555) add support for filtering/paging/sorting options for "list_**" methods (Azure#18302) [Tables] Misc client updates (Azure#18462) [EventHub&ServiceBus] Prepare for release (Azure#18527) [Communication]: Updated communication connection strings to be consistent across packages and languages (Azure#18519) [AppConfig] Fixing samples (Azure#18542) ...

…into agrifood_nspkg * 'master' of https://github.com/Azure/azure-sdk-for-python: (31 commits) [purview] add catalog client (Azure#17788) Add spellcheck for modified files (Azure#18496) [Container Registry] Update API for Beta 2 (Azure#18392) First version of Confidential Ledger Python SDK (Azure#17951) [llc] add quickstart (Azure#18537) [Storage][Fix]Copy source is redirecting (Azure#18577) [Tables] pre-release script (Azure#18505) Update changelogs (Azure#18575) [formrecognizer] Adding to_dict() on custom models (Azure#18402) Raise CredentialUnavailableError when CLI subprocess times out (Azure#18509) Extend Check Enforcer timeout (Azure#18526) Core raw streaming (Azure#17920) hide secrets in mgmt sdk (Azure#18535) add filter samples for list methods (Azure#18480) Revert changes to SetDevVersion. (Azure#18555) add support for filtering/paging/sorting options for "list_**" methods (Azure#18302) [Tables] Misc client updates (Azure#18462) [EventHub&ServiceBus] Prepare for release (Azure#18527) [Communication]: Updated communication connection strings to be consistent across packages and languages (Azure#18519) [AppConfig] Fixing samples (Azure#18542) ...

seankane-msft added 8 commits April 27, 2021 17:56

changing ContainerRepositoryClient to ContainerRepository

d7bcfe6

renaming files

7a0b598

re-recording, commenting out tests that are not necessary

15bbaaf

working sync registry artifact class

b91302c

async registry artifact

6f7c55c

issue with recording infra that is removing an acr specific oauth path

22ecb18

pylint issues

d5986dd

recording and processors

1287136

seankane-msft added the Container Registry label Apr 28, 2021

seankane-msft requested review from annatisch, iscai-msft, kristapratico, mccoyp and swathipil April 28, 2021 18:10

seankane-msft self-assigned this Apr 28, 2021

seankane-msft added 15 commits April 28, 2021 14:12

removing commented out code

145949b

undoing changes to cache

09f3636

more lint fixes

6e38bf1

help with logging output

d9659c0

change to list_repository_names

941473a

renaming for consistency

44ceace

all changes made, plus recordings

4e5dbc8

fixing up more tests again!

f7caebd

formatting

6ced986

merge conflicts

5417f1d

fixing up merge issues

83c0d90

merge conflicts

c0ed9ff

more conflicts

ed3f937

undoing changes to gen code

ccec23b

pylint issues

afa0c1b

seankane-msft mentioned this pull request May 5, 2021

[Container Registry] renamings #18492

Merged

annatisch reviewed May 5, 2021

View reviewed changes

seankane-msft added 5 commits May 5, 2021 17:00

working through massive merge conflicts

a05639a

renaming containerrepository.repository to name

91f150f

renaming enums, dropping the 'By' off the end

4af08b9

adding pipeline wrappers

992df39

lint fixes

35eb6e5

seankane-msft mentioned this pull request May 5, 2021

Standardize on repositoryName parameter in ContainerRegistryClient methods #18503

Closed

seankane-msft linked an issue May 5, 2021 that may be closed by this pull request

Standardize on repositoryName parameter in ContainerRegistryClient methods #18503

Closed

removing unused import

fd701a4

Azure deleted a comment from check-enforcer bot May 6, 2021

seankane-msft mentioned this pull request May 6, 2021

[Container Registry] Acr enums #18400

Merged

seankane-msft added this to the [2021] May milestone May 6, 2021

seankane-msft requested a review from annatisch May 6, 2021 16:08

seankane-msft and others added 4 commits May 6, 2021 15:43

Merge branch 'master' into acr-container-repo

cb3341e

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …

c25fec2

…into acr-container-repo

fixing an improper merge conflict in gen code

91e9839

checking out from master

ba7d4da

seankane-msft mentioned this pull request May 6, 2021

[Container Registry] Anonymous Access Client #18550

Merged

adding the pipeline wrappers

8ef7421

annatisch approved these changes May 7, 2021

View reviewed changes

few comments

2ae91f1

seankane-msft added the auto-merge label May 7, 2021

final newline misssing

9c20c59

Azure deleted a comment from check-enforcer bot May 7, 2021

ghost merged commit 9fba82f into Azure:master May 7, 2021

seankane-msft deleted the acr-container-repo branch May 7, 2021 22:16

This pull request was closed.

		from ._models import ContentProperties


		class ContainerRepository(ContainerRegistryBaseClient):



		class RegistryArtifactOrderBy(str, Enum):
		class ManifestOrderBy(str, Enum):

Comments

Conversation

seankane-msft commented Apr 28, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented May 7, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.