Skip to content
This repository has been archived by the owner on Jul 14, 2023. It is now read-only.

[Custom] Optimize custom function #872

Merged
merged 4 commits into from
Aug 9, 2021

Conversation

kairu-ms
Copy link
Contributor

@kairu-ms kairu-ms commented Aug 4, 2021

Closed #860

Add if clause for optional parameters.

If the argument is optional, it will be assigned as parameter property when it is not None.

Previous Code

def videoanalyzer_video_create(client,
                               resource_group_name,
                               account_name,
                               video_name,
                               title=None,
                               description=None):
    parameters = {}
    parameters['title'] = title
    parameters['description'] = description
    return client.create_or_update(resource_group_name=resource_group_name,
                                   account_name=account_name,
                                   video_name=video_name,
                                   parameters=parameters)

Current Code

def videoanalyzer_video_create(client,
                               resource_group_name,
                               account_name,
                               video_name,
                               title=None,
                               description=None):
    parameters = {}
    if title is not None:
        parameters['title'] = title
    if description is not None:
        parameters['description'] = description
    return client.create_or_update(resource_group_name=resource_group_name,
                                   account_name=account_name,
                                   video_name=video_name,
                                   parameters=parameters)

Add logic to remove empty property of parameter.

If a property of parameter is optional and it is an empty dict, it will be removed from parameter.

Previous Code

def videoanalyzer_video_analyzer_create(client,
                                        resource_group_name,
                                        account_name,
                                        type_=None,
                                        user_assigned_identity=None,
                                        key_identifier=None,
                                        video_analyzer_identity_type=None,
                                        user_assigned_identities=None):
    parameters = {}

    parameters['encryption'] = {}
    parameters['encryption']['type'] = type_
    parameters['encryption']['identity'] = {}
    parameters['encryption']['identity']['user_assigned_identity'] = user_assigned_identity
    parameters['encryption']['key_vault_properties'] = {}
    parameters['encryption']['key_vault_properties']['key_identifier'] = key_identifier
    parameters['identity'] = {}
    parameters['identity']['type'] = video_analyzer_identity_type
    parameters['identity']['user_assigned_identities'] = user_assigned_identities
    return client.create_or_update(resource_group_name=resource_group_name,
                                   account_name=account_name,
                                   parameters=parameters)

Current Code

def videoanalyzer_video_analyzer_create(client,
                                        resource_group_name,
                                        account_name,
                                        type_=None,
                                        user_assigned_identity=None,
                                        key_identifier=None,
                                        video_analyzer_identity_type=None,
                                        user_assigned_identities=None):
    parameters = {}

    parameters['encryption'] = {}
    if type_ is not None:
        parameters['encryption']['type'] = type_
    parameters['encryption']['identity'] = {}
    if user_assigned_identity is not None:
        parameters['encryption']['identity']['user_assigned_identity'] = user_assigned_identity
    if len(parameters['encryption']['identity']) == 0:
        del parameters['encryption']['identity']
    parameters['encryption']['key_vault_properties'] = {}
    if key_identifier is not None:
        parameters['encryption']['key_vault_properties']['key_identifier'] = key_identifier
    if len(parameters['encryption']['key_vault_properties']) == 0:
        del parameters['encryption']['key_vault_properties']
    if len(parameters['encryption']) == 0:
        del parameters['encryption']

    parameters['identity'] = {}
    if video_analyzer_identity_type is not None:
        parameters['identity']['type'] = video_analyzer_identity_type
    if user_assigned_identities is not None:
        parameters['identity']['user_assigned_identities'] = user_assigned_identities
    if len(parameters['identity']) == 0:
        del parameters['identity']

    return client.create_or_update(resource_group_name=resource_group_name,
                                   account_name=account_name,
                                   parameters=parameters)

@kairu-ms kairu-ms requested a review from qiaozha August 4, 2021 07:00
@kairu-ms kairu-ms self-assigned this Aug 4, 2021
@kairu-ms kairu-ms requested a review from jsntcy August 4, 2021 07:01
@kairu-ms kairu-ms force-pushed the fix-custom-function branch from c3370f3 to 29ee927 Compare August 4, 2021 07:54
if keys is not None:
creation_params['properties']['policy_signing_certificates']['keys'] = keys
if len(creation_params['properties']['policy_signing_certificates']) == 0:
del creation_params['properties']['policy_signing_certificates']
Copy link
Member

Choose a reason for hiding this comment

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

how about the length of properties.

factory['identity'] = {}
factory['identity']['type'] = "SystemAssigned"
if len(factory['identity']) == 0:
del factory['identity']
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is necessary given that identity must have a type with value "SystemAssigned" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. But it is hard to track this case.

if user_assigned_identities is not None:
parameters['identity']['user_assigned_identities'] = user_assigned_identities
if len(parameters['identity']) == 0:
del parameters['identity']
Copy link
Member

Choose a reason for hiding this comment

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

how about add some common logic to check all the empty dict in the build result instead of adding so much if XXX is not None. Is it more easier to do ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is verbose. But I found it's hard to implement an easier way in current logic.

@kairu-ms kairu-ms requested a review from yonzhan August 5, 2021 01:52
@kairu-ms kairu-ms force-pushed the fix-custom-function branch 2 times, most recently from 6c9dfb5 to 39ac161 Compare August 6, 2021 07:51
@kairu-ms kairu-ms force-pushed the fix-custom-function branch from 39ac161 to 7747f08 Compare August 6, 2021 07:54
@kairu-ms kairu-ms force-pushed the fix-custom-function branch from f6d3f86 to be66820 Compare August 6, 2021 08:37
@kairu-ms kairu-ms merged commit 2352caf into Azure:master Aug 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug for initializing JSON objects when children are empty
3 participants