-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[ACS] Provide a short name alias for the orchestrator type flag #2553
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
Conversation
This PR simply adds a short name alias to the `--orchestrator-type` flag, since it is very common to set
Codecov Report
@@ Coverage Diff @@
## master #2553 +/- ##
==========================================
+ Coverage 72.13% 72.16% +0.03%
==========================================
Files 362 362
Lines 19776 19798 +22
Branches 2920 2920
==========================================
+ Hits 14266 14288 +22
- Misses 4589 4593 +4
+ Partials 921 917 -4
Continue to review full report at Codecov.
|
| register_cli_argument('acs', 'resource_group', arg_type=resource_group_name_type) | ||
|
|
||
| register_cli_argument('acs', 'orchestrator_type', **enum_choice_list(ContainerServiceOchestratorTypes)) | ||
| register_cli_argument('acs', 'orchestrator_type', options_list=('--orchestrator_type', '-t'), **enum_choice_list(ContainerServiceOchestratorTypes)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the long name simply be --type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@troydai I would totally be in favor of that! I just wasn't sure if that would be a breaking change worth doing (did the ACS module GA already?). Let me know what you think and I'm happy to make that change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right. We can't do this breaking change. Ideally, we can do multiple aliases for one parameter, however this would further confusing the namespace. Let's keep the name for now. We can think of a way to introduce obsoleted parameter if necessary.
In addition, the parameter should be registered as --orchestrator-type instead of --orchestrator_type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, I didn't realize I introduced that typo :) Fixed!
* Azure/master: (478 commits) vm live test: allow more valid power states on vmss test verifications (Azure#2564) rbac:catch more graph error (Azure#2567) appservice: support to create plan when create a webapp (Azure#2550) Update storage tests (Azure#2556) Change PEP8 check filter from whitelist to blacklist (Azure#2557) Add scenario tests documentation (Azure#2555) [ACS] Adding support for configuring a default ACS cluster (Azure#2554) [ACS] Provide a short name alias for the orchestrator type flag (Azure#2553) Sql Import/Export CLI commands and test (Azure#2538) Fix format bug. (Azure#2549) [VM/VMSS] Improved disk caching support (Azure#2522) VM/VMSS: incorporate credentials validation logic used by portal (Azure#2537) Script that creates packaged releases package archive (Azure#2508) Adding alias for defaults flag (Azure#2540) Add wait commands and --no-wait support (Azure#2524) choice list outside of named arguments (Azure#2521) Fixed test failure in test_sql_db_mgmt. (Azure#2530) core: support login using service principal with a cert (Azure#2457) Add note about being in preview (Azure#2512) vm:fix distro check mechanism used by disk encryption (Azure#2511) ...
This PR simply adds a short name alias to the
--orchestrator-typeflag (on theaz acs createcommand), since it is very common to set.