search

microsoft / knack

Knack - A Python command line interface framework
https://pypi.python.org/pypi/knack
MIT License
347 stars 95 forks source link

fix default value set for bool type #273

Closed AllyW closed 1 year ago

AllyW commented 1 year ago

bool is subclass of int, while default True or False should not be converted into int 1 or 0. Otherwise, type check would fail for aaz cmds (https://github.com/Azure/azure-cli/blob/58ac6ab07fa850e1a6458dec87af91cd6725069e/src/azure-cli-core/azure/cli/core/aaz/_field_type.py#L47)

AllyW commented 1 year ago

in this pr: https://github.com/Azure/azure-cli/pull/26819 aaz cmd: image

before: image after: image

AllyW commented 1 year ago

corresponding swagger line： https://github.com/Azure/azure-rest-api-specs/blob/493aa7224fd65fe1e5b4cff59bcae5c6cdf4525e/specification/monitor/resource-manager/Microsoft.Insights/preview/2023-05-01-preview/tenantActionGroups_API.json#L570

jiasli commented 1 year ago

Also see https://stackoverflow.com/questions/37888620/comparing-boolean-and-int-using-isinstance

jiasli commented 1 year ago

As a side note, https://github.com/Azure/azure-cli/pull/16081 took a different approach. It sets the default value of role to None and defaulting it to Contributor manually in custom.py's code.

jiasli commented 1 year ago

The C source code of bool also mentions:

https://github.com/python/cpython/blob/832c37d42a395d4ea45994daffa5e41bd74ac1bb/Objects/boolobject.c#L118

The class bool is a subclass of the class int, and cannot be subclassed.

An alternative is to consider developing our own Bool and implementing special methods such as __bool__, __str__, so that it acts like a real bool, and also bears additional attribute is_default.