fix default value set for bool type#273
Conversation
|
in this pr: Azure/azure-cli#26819 before: |
knack/commands.py
Outdated
| elif isinstance(arg_default, bool): | ||
| pass |
There was a problem hiding this comment.
I don't think we can simply pass here, as this will make it impossible to distinguish whether the received True is user specified or the default value.
There was a problem hiding this comment.
I think it's useless, because it only added is_default property for str and int, not cover the other types, such as bool, float, timestamp etc.
There was a problem hiding this comment.
BTW, I also found the following lines has a bug when the arg_default is 0 int value
Lines 129 to 131 in e496c95
There was a problem hiding this comment.
Could you please elaborate what the bug is?
There was a problem hiding this comment.
After reviewing the code, I feel this if arg_default: statement is deliberately added in order to ignore False/0 as default.
For example, for arguments --disable-integrity-monitoring and --disable-integrity-monitoring-autoupgrade of az vm create, their default values are False/0, so they are deliberately omitted from the help message:
> az vm create -h
...
--disable-integrity-monitoring : Disable the default behavior of installing
guest attestation extension and enabling
System Assigned Identity for Trusted
Launch enabled VMs and VMSS.
--disable-integrity-monitoring-autoupgrade : Disable auto upgrade of guest attestation
extension for Trusted Launch enabled VMs
and VMSS.
knack/commands.py
Outdated
| # adjust here is because: | ||
| # 1) bool is subclass of int so isinstance(True, int) cannot distinguish between int and bool | ||
| # 2) bool is not extendable according to | ||
| # https://stackoverflow.com/questions/2172189/why-i-cant-extend-bool-in-python, | ||
| # so bool's is_default is ignored for now |
There was a problem hiding this comment.
Code comment should usually not focus on adjustment, but the status quo.
We can get the short link for a Stackoverflow post via the "share" button:
| # adjust here is because: | |
| # 1) bool is subclass of int so isinstance(True, int) cannot distinguish between int and bool | |
| # 2) bool is not extendable according to | |
| # https://stackoverflow.com/questions/2172189/why-i-cant-extend-bool-in-python, | |
| # so bool's is_default is ignored for now | |
| # bool's is_default is ignored for now, because | |
| # 1. bool is a subclass of int, so isinstance(True, int) cannot distinguish between int and bool | |
| # 2. bool is not extendable according to https://stackoverflow.com/a/2172204/2199657 |
|
As a side note, Azure/azure-cli#16081 took a different approach. It sets the default value of |
|
The C source code of
An alternative is to consider developing our own |
5 participants
boolis subclass of int, while defaultTrueorFalseshould not be converted into int1or0.Otherwise, type check would fail for
aazcmds (https://github.com/Azure/azure-cli/blob/58ac6ab07fa850e1a6458dec87af91cd6725069e/src/azure-cli-core/azure/cli/core/aaz/_field_type.py#L47)