Skip to content

[Storage] Fix #14054: 'NoneType' object has no attribute '__name__' #16993

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.

Sign up for GitHub

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

Merged
evelyn-ys merged 1 commit into from
Feb 22, 2021
Merged

[Storage] Fix #14054: 'NoneType' object has no attribute '__name__' #16993

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/azure-cli/azure/cli/command_modules/storage/sdkutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ def get_table_data_type(cli_ctx, module_name, *type_names):

def get_blob_service_by_type(cli_ctx, blob_type):
type_to_service = {
'block': lambda ctx: get_sdk(ctx, ResourceType.DATA_STORAGE, 'BlockBlobService', mod='blob'),
'page': lambda ctx: get_sdk(ctx, ResourceType.DATA_STORAGE, 'PageBlobService', mod='blob'),
'append': lambda ctx: get_sdk(ctx, ResourceType.DATA_STORAGE, 'AppendBlobService', mod='blob')
'block': lambda ctx: get_sdk(ctx, ResourceType.DATA_STORAGE, 'BlockBlobService', mod='blob', checked=False),
'page': lambda ctx: get_sdk(ctx, ResourceType.DATA_STORAGE, 'PageBlobService', mod='blob', checked=False),
'append': lambda ctx: get_sdk(ctx, ResourceType.DATA_STORAGE, 'AppendBlobService', mod='blob', checked=False)
Comment on lines +28 to +30
Copy link
Member

@jiasli jiasli Feb 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I would really prefer to make checked=False the default for get_sdk, otherwise it is not universal and 'NoneType' object has no attribute '__name__' will continue to happen in other command modules. (#15776 (comment))

Copy link
Member

@jiasli jiasli Feb 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I already made checked=False the default for _get_attr in #14690

azure-cli/src/azure-cli-core/azure/cli/core/profiles/_shared.py

Lines 534 to 535 in 301ba06

def _get_attr(sdk_path, mod_attr_path, checked=False):
"""If `checked` is True, None is returned in case of import failure."""

but it only applies to get_client_class which calls _get_attr without checked:

azure-cli/src/azure-cli-core/azure/cli/core/profiles/_shared.py

Lines 552 to 553 in 301ba06

def get_client_class(resource_type):
return _get_attr(resource_type.import_prefix, '#' + resource_type.client_name)

get_versioned_sdk will default checked to True via

azure-cli/src/azure-cli-core/azure/cli/core/profiles/_shared.py

Lines 573 to 574 in 301ba06

def get_versioned_sdk(api_profile, resource_type, *attr_args, **kwargs):
checked = kwargs.get('checked', True)

azure-cli/src/azure-cli-core/azure/cli/core/profiles/_shared.py

Line 585 in 301ba06

loaded_obj = _get_attr(sdk_path, mod_attr_path, checked)

Copy link
Member Author

@evelyn-ys evelyn-ys Feb 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Personally I totally agree with you. It's just I was afraid of making great changes to all modules. Should we discuss whether to change the behavior or not with all module owners?

Copy link
Member

@jiasli jiasli Feb 19, 2021

Choose a reason for hiding this comment

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

We can go ahead and make the change. If CI passes, we are fine.

Copy link
Member

@jiasli jiasli Feb 19, 2021

Choose a reason for hiding this comment

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

Python's import statement raises ImportError in case of non-existing module.

Also, checking whether the result is None is against the look before you leap practice.

}

try:
Expand Down