azure-sdk-for-python icon indicating copy to clipboard operation
azure-sdk-for-python copied to clipboard

Azure.KeyVault SDK models lacks serialization methods

Open logachev opened this issue 3 years ago • 12 comments

  • azure_keyvault_keys:
  • 4.3.1:
  • Linux:
  • 3.8.5:

Describe the bug Latest version of Azure.Keyvault SDKs don't have serialization in the models.

Models in this file don't inherit msrest, so there is no easy serialization available. https://github.com/Azure/azure-sdk-for-python/blob/master/sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/_models.py

Same goes to Secrets and Certificates.

To Reproduce Steps to reproduce the behavior: 1.

Expected behavior Azure Python SDK has consistent experience and all models have serialization available.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

logachev avatar Mar 05 '21 02:03 logachev

Thanks @logachev for the feedback! Adding @mccoyp and @chlowell who can comment further on the changes in KeyVault-Keys specifically. Was this a change in the models in a recent release?

I'm curious about your use of the serialization of these models, which methods from the msrest base Model type were you making use of? And in what context? The presence of the msrest serialization typically indicates that these are part of auto-generated code - whereas many hand-coded models do not inherit from the msrest base. We've been discussing this topic a bit internally, so I'm really curious to hear more about the scenario where you're using this serialization. FYI @johanste, @lmazuel

annatisch avatar Mar 05 '21 03:03 annatisch

These models are handwritten and have never inherited Model or exposed serialization methods. The prior (track 1) azure-keyvault package's KeyVaultClient returned auto-generated Models, so there could be a migration issue here. I share Anna's curiousity about the specific scenario.

chlowell avatar Mar 05 '21 16:03 chlowell

Yeah, this is an update scenario from prior azure-keyvault package.. https://github.com/cloud-custodian/cloud-custodian

Primarily we need it to transform resource to the dict for further usage. The tool itself doesn't know details about KeyVault objects, but it requires common representation for all Azure resource objects and dict suits this for out use case.

Another scenario is just saving Azure object info into a file. At the moment, json.dumps() can't handle KeyVault objects.

logachev avatar Mar 05 '21 17:03 logachev

@chlowell - for some of the SDKs (e.g. ServiceBus, Storage), we give the handcoded models a dictionary interface to facilitate passing the objects around where dicts are supports. Maybe something like that could be useful here?

Of course, that doesn't yet resolve the json.dumps issue, as that will also require serialization of complex types (e.g. datetimes)

annatisch avatar Mar 05 '21 21:03 annatisch

@annatisch yeah, I think it could help.. For our usecase, it is sufficient to get a dict parsed from the raw REST response.. Is there a way to retrieve it?

logachev avatar Mar 07 '21 06:03 logachev

Thanks @logachev - yes you should be able to access the raw REST response. Our newer SDKs all support the raw_response_hook keyword argument (which can be passed in either once, at the client constructor, or per operation for which it is needed). This takes a function which accepts a single argument, an instance of PipelineResonse, which contains instances of both the HttpRequest and HttpResponse objects. This should allow you to access the response data. In addition, there's also a raw_request_hook argument as well, if you also need to see the serialized outgoing data.

A snippet using this callback might look something like:


def get_response_data(pipeline):
    data = pipline.http_response.body()  # Note that this function is awaitable for async clients
    data_as_dict = json.loads(data)
    store_data(data_as_dict)  # do something with the response dict


client.get_key("key-name", raw_response_hook=get_response_data)

annatisch avatar Mar 07 '21 18:03 annatisch

@annatisch I'll need a few days to try this approach, but it looks promising.. :)

Also, I think part of my feedback is lack of consistency.. I know Track 2 was supposed to bring unified approach to the SDKs and for some things I feel like it is amazing.. However, end users don't really follow what models are handcrafted and what models are autogenerated.. It would be great if there was some baseline that is consistent across mgmt and data plane SDKs!

logachev avatar Mar 08 '21 19:03 logachev

We have discussion in progress with @johanste to get an easy standardized way to get dict representation of all models. Totally legit feedback, let's talk about doing it.

lmazuel avatar Apr 02 '21 17:04 lmazuel

Is there any process or associate PR about this issue?

Codejune avatar Sep 22 '21 13:09 Codejune

We recently added a custom JSONEncoder to azure-core, which will help us serialize models throughout the SDK (and was partially motivated by this issue). This PR is tracking the effort to add serialization to Key Vault models, which I hope to get done in a beta release after the next GA of KV packages.

mccoyp avatar Sep 22 '21 18:09 mccoyp

@mccoyp I can't tell from the issue but is this resolved with the PR that was merged? Can you confirm?

Petermarcu avatar Aug 09 '22 04:08 Petermarcu

@Petermarcu the linked PR will help with this issue, but the serialization story hasn't been resolved yet. I'll bump the priority of this work; thanks for double-checking 🙂

mccoyp avatar Aug 12 '22 00:08 mccoyp

Hi @logachev, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

github-actions[bot] avatar Mar 15 '24 18:03 github-actions[bot]