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

{Core} Add dict transformation for typespec generated SDKs #30339

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/azure-cli-core/azure/cli/core/commands/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from knack.preview import ImplicitPreviewItem, PreviewItem, resolve_preview_info
from knack.experimental import ImplicitExperimentalItem, ExperimentalItem, resolve_experimental_info
from knack.log import get_logger, CLILogging
from knack.util import CLIError, CommandResultItem, todict
from knack.util import CLIError, CommandResultItem
from knack.events import EVENT_INVOKER_TRANSFORM_RESULT
from knack.validators import DefaultStr

Expand Down Expand Up @@ -715,6 +715,7 @@ def _run_job(self, expanded_arg, cmd_copy):
elif _is_paged(result):
result = list(result)

from ..util import todict
result = todict(result, AzCliCommandInvoker.remove_additional_prop_layer)

event_data = {'result': result}
Expand Down
36 changes: 35 additions & 1 deletion src/azure-cli-core/azure/cli/core/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from urllib.request import urlopen

from knack.log import get_logger
from knack.util import CLIError, to_snake_case
from knack.util import CLIError, to_snake_case, to_camel_case

logger = get_logger(__name__)

Expand Down Expand Up @@ -624,6 +624,40 @@ def b64_to_hex(s):
return hex_data


def todict(obj, post_processor=None):
"""
Convert an object to a dictionary. Use 'post_processor(original_obj, dictionary)' to update the
dictionary in the process
"""
from datetime import date, time, datetime, timedelta
from enum import Enum
if isinstance(obj, dict):
result = {k: todict(v, post_processor) for (k, v) in obj.items()}
return post_processor(obj, result) if post_processor else result
if isinstance(obj, list):
return [todict(a, post_processor) for a in obj]
if isinstance(obj, Enum):
return obj.value
if isinstance(obj, (date, time, datetime)):
return obj.isoformat()
if isinstance(obj, timedelta):
return str(obj)
# This is the only difference with knack.util.todict because for typespec generated SDKs
# The base model stores data in obj.__dict__['_data'] instead of in obj.__dict__
# We need to call obj.as_dict() to extract data for this kind of model
if hasattr(obj, 'as_dict') and not hasattr(obj, '_attribute_map'):
Copy link
Member

Choose a reason for hiding this comment

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

Another approach to detect if obj is an SDK model is something similar to

type(obj).__base__.__name__ == 'Model'

str(type(obj).__base__) returns

"<class 'azure.cli.command_modules.keyvault.vendored_sdks.azure_keyvault_securitydomain._model_base.Model'>"

However, this is also not very reliable as the name Model may change at any time.

Using isinstance() is impossible as Model is defined in each SDK, such as azure.cli.command_modules.keyvault.vendored_sdks.azure_keyvault_securitydomain._model_base.Model, instead of in azure.core. There is no single Model that is the base class of all SDK objects.

Also, using isinstance() requires importing the Model class which impairs the performance.

As an example, remove_additional_prop_layer previously imports Model with

from msrest.serialization import Model

#13843 changed it to use a try... catch... strategy:

@staticmethod
def remove_additional_prop_layer(obj, converted_dic):
# Follow EAFP to flatten `additional_properties` auto-generated by SDK
# See https://docs.python.org/3/glossary.html#term-eafp
try:
if 'additionalProperties' in converted_dic and isinstance(obj.additional_properties, dict):
converted_dic.update(converted_dic.pop('additionalProperties'))
except AttributeError:
pass
return converted_dic

Thus, there is no need to import Model for non-Model objects.

result = {to_camel_case(k): todict(v, post_processor) for k, v in obj.as_dict().items()}
return post_processor(obj, result) if post_processor else result
Comment on lines +648 to +650
Copy link
Member

Choose a reason for hiding this comment

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

A good question from @kairu-ms, what if the REST spec defines a asDict property?

Copy link
Member

Choose a reason for hiding this comment

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

For now there is no such a property: https://github.com/search?q=repo%3AAzure%2Fazure-rest-api-specs%20asDict&type=code. If there is one in the future, we could rename it.

if hasattr(obj, '_asdict'):
return todict(obj._asdict(), post_processor)
if hasattr(obj, '__dict__'):
result = {to_camel_case(k): todict(v, post_processor)
for k, v in obj.__dict__.items()
if not callable(v) and not k.startswith('_')}
return post_processor(obj, result) if post_processor else result
return obj


def random_string(length=16, force_lower=False, digits_only=False):
from string import ascii_letters, digits, ascii_lowercase
from random import choice
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,13 +474,3 @@ def transform_certificate_issuer_admin_list(result, **command_args):
} for contact in admin_contacts]
return ret
return result


def transform_security_domain_output(result, **command_args):
if not result or isinstance(result, dict):
return result
ret = {
'status': getattr(result, 'status', None),
'statusDetails': getattr(result, 'status_details', None)
}
return ret
10 changes: 4 additions & 6 deletions src/azure-cli/azure/cli/command_modules/keyvault/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
get_client, get_client_factory, Clients, is_azure_stack_profile)

from azure.cli.command_modules.keyvault._transformers import (
filter_out_managed_resources, transform_security_domain_output,
filter_out_managed_resources,
multi_transformers, transform_key_decryption_output, keep_max_results, transform_key_list_output,
transform_key_output, transform_key_encryption_output, transform_key_random_output,
transform_secret_list, transform_deleted_secret_list, transform_secret_set,
Expand Down Expand Up @@ -145,11 +145,9 @@ def load_command_table(self, _):
with self.command_group('keyvault security-domain', data_security_domain_entity.command_type) as g:
g.keyvault_custom('init-recovery', 'security_domain_init_recovery')
g.keyvault_custom('restore-blob', 'security_domain_restore_blob')
g.keyvault_custom('upload', 'security_domain_upload', supports_no_wait=True,
transform=transform_security_domain_output)
g.keyvault_custom('download', 'security_domain_download', supports_no_wait=True,
transform=transform_security_domain_output)
g.keyvault_custom('wait', '_wait_security_domain_operation', transform=transform_security_domain_output)
g.keyvault_custom('upload', 'security_domain_upload', supports_no_wait=True)
g.keyvault_custom('download', 'security_domain_download', supports_no_wait=True)
g.keyvault_custom('wait', '_wait_security_domain_operation')

with self.command_group('keyvault key', data_key_entity.command_type) as g:
g.keyvault_custom('create', 'create_key', transform=transform_key_output, validator=validate_key_create)
Expand Down