Add docs for az lock update. #2702

brendandburns · 2017-03-31T23:24:36Z

Part of #2694

brendandburns · 2017-03-31T23:24:48Z

tjprescott

Also, there's an unresolved Pylint issue.

tjprescott · 2017-04-03T16:08:29Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/_params.py

@@ -79,3 +79,4 @@

 register_cli_argument('lock', 'name', options_list=('--name', '-n'))
 register_cli_argument('lock create', 'level', options_list=('--lock-type', '-t'), required=True, **enum_choice_list(LockLevel))
+register_cli_argument('lock update', 'level', options_list=('--lock-type', '-t'), required=True, **enum_choice_list(LockLevel))


These two lines should be consolidated:

register_cli_argument('lock', 'level', options_list=('--lock-type', '-t'), **enum_choice_list(LockLevel))

Essentially, you scope it to "lock" and remove the "requiredness". The CLI will determine this automatically by examining the respective method signatures.

tjprescott · 2017-04-03T16:09:38Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/custom.py

+    :param name: The name of the lock
+    :type name: str
+    :param resource_group_name: The name of the resource group, either the lock is on this group, or a
+    resource in this group.


It sounds like resource_group_name should be required and not have a default value of None. This is another issue pointed out in #2694. It does not seem like you could identify any lock with just the name (unless they are globally unique).

Updated, you can have subscription level locks (which must be unique for a subscription)

Ah, I didn't realize that. Does it pretty much prevent you from deleting anything?

tjprescott · 2017-04-03T16:15:48Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/custom.py

+    :param resource_group_name: The name of the resource group, either the lock is on this group, or a
+    resource in this group.
+    :type resource_group_name str
+    :param notes: Notes about this lock [optional]


You don't need [optional] here as it is an optional parameter.

tjprescott · 2017-04-03T16:21:30Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/custom.py

-                level=None, notes=None, lock_id=None, lock_type=None):
+def update_lock(name, resource_group_name=None, level=None, notes=None):
+    '''
+    :param name: The name of the lock


Please move this docstring help into the _help.py files. The reason is because the live doc's Edit button points directly to that file, making it easy for someone to edit the help in the future.

brendandburns · 2017-04-03T23:11:52Z

Comments addressed, please re-check.
Thanks!

tjprescott · 2017-04-03T23:45:08Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/_help.py

@@ -10,6 +10,15 @@
    type: group
    short-summary: Manage Azure locks.
 """
+helps['lock update'] = """


This should be in the YAML format like the rest of the entries in the file. I'm actually kind of surprised it doesn't throw an error. The long description piece would be a good place to note that locks can apply to a subscription/resource group/resource.

Follow-up: it does throw an error. That's your CI error.

fixed. (as an end-user, I found the syntax/errors are semi-inscrutable fwiw)

codecov-io · 2017-04-05T04:08:01Z

Codecov Report

Merging #2702 into master will increase coverage by <.01%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master    #2702      +/-   ##
==========================================
+ Coverage   62.87%   62.87%   +<.01%     
==========================================
  Files         480      480              
  Lines       25807    25808       +1     
  Branches     3905     3905              
==========================================
+ Hits        16226    16227       +1     
  Misses       8572     8572              
  Partials     1009     1009

Impacted Files	Coverage Δ
...urce/azure/cli/command_modules/resource/_params.py	`100% <100%> (ø)`	⬆️
...source/azure/cli/command_modules/resource/_help.py	`100% <100%> (ø)`	⬆️
...ource/azure/cli/command_modules/resource/custom.py	`51.14% <33.33%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa855de...09aca4b. Read the comment docs.

tjprescott · 2017-04-05T16:13:03Z

src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/_help.py

+    examples:
+        - name: Update a lock with new notes
+          text: >
+            az lock update --name lockName --resource-group-name group --notes newNotesHere


Resource group name is globally aliased as --resource-group. Is this a typo?

yep, fixed.

brendandburns · 2017-04-05T23:09:00Z

Rebased and comments addressed.

Please take another look.

* Enable delay-load of descriptions for commands (speed up az) * Update find indexing commands to accept callables for description. * Command load time in progress * - Moved previously dead command filter from parser to application configuration. - Removed unused configuration object/argv on application create. * Remove unused argument (pylint) * Remove dummy parameter * Fix for python 2.7 * Fix yet incorrect passage of parameters * Fix up additional pylint complaints * Update tests * Update tests * Fix up more tests * Fix up more core tests * Enable delay-load of descriptions for commands (speed up az) * Update find indexing commands to accept callables for description. * [Network] Remove nulls from VPN connection show/list output (#2748) * Fix #1615. * Code review feedback. * Update test docs for running individual test and all tests in mod (#2763) * Update test docs for running individual test and all tests in mod * Made feedback changes * Make argument parameters match up. (#2717) Make lock command parameter aliases match up with resource commands. * [DevTestLabs] Adding scenario test to create simple Linux + Windows VM in lab (#2767) * WIP create linux + Windows vm in lab * Adding recording * Add some more error checking/handling. (#2768) Add more validation to resolve "lock level" for lock commands. * Fix doc references to azure.cli.commands (#2740) * Fix doc references to azure.cli.commands This module has moved to azure.cli.core.commands * Fix PyLint * Add clearer guidelines on modifying changelog (#2739) * Add clearer guidelines on modifying changelog * A few smaller changes * another small format change * Code review changes * [DevTestLabs] Exposing commands to manage secrets in the lab (#2691) * ACS Update: nulling out the windows profile so that there isn't a validation fail… (#2764) * nulling out the windows profile so that there isn't a valdiation failure for missing password ACS doesn't return a password on GET. az acs scale command does a GET then PUT, but since ACS doesn't return the password the verification is failing before the PUT is sent to ACS. There is a bug in ACS this exposes. So this shouldn't be merged until after the ACS rollout finishes. Should be about start of next week. * updating history * updating version in history * removing white space added by editor * [Compute] Fix issues with VMSS and VM availability set update. (#2773) * Fix issues with VMSS and VM availability set update. * Update help. Fix #2762. * Error out if you try to list resources for a group that doesn't exist. (#2769) * Minor text fixes (#2776) * Add docs for az lock update. (#2702) * [DevTestLab] Explicitly enable usage of saved secrets while lab vm creation (#2686) * Explicitly enable usage of saved secrets for vm creation * Better error message with not overriding competing paramters * Adding export-artifacts commands on formula (#2707) * core: apply configured defaults on optional arg (#2770) * Core:apply configured defaults on optional argument * add a test * add tests * update history doc * address review feedback * [Network] Support active-active VNet gateways (#2751) * Start active-active test scenario. * Add active-active parameter. * Active-active scenario test 1 (cross premise) * Add second active-active scenario (vnet-to-vnet) * Refine active-active gateway configuration. * Pylint... * Code review feedback * Packaged release notes and changes for 0.2.4 (#2735) * Modify HISTORY.md * Update Dockerfile * Update debian also * Add pip dependencies also * Command load time in progress * - Moved previously dead command filter from parser to application configuration. - Removed unused configuration object/argv on application create. * Remove unused argument (pylint) * Remove dummy parameter * Fix for python 2.7 * Fix yet incorrect passage of parameters * Fix up additional pylint complaints * Update tests * Update tests * Fix up more tests * Fix up more core tests * Improve load time of custom.py for profile, find and configure (speeds up raw az command) * Pylint + flake8 fixes * Fix new vm tests that failed due to perf refactoring * Update redis tests that was broken due to perf refactoring * Delay-load msrest for command executions that don't need it * Fix flake8 issues * Fixing/improving detection of pageable class * flake8 fixes * Fix broken merge from upstream/master * Fix broken merge (again) * flake8 fixes * Fix up even more merge errors from last upstream merge * Flake8 fixes (wrong number of newlines) * Fix delay load of storage assembly for az * Update history to reference improved performance

msftclas added the cla-not-required label Mar 31, 2017

tjprescott suggested changes Apr 3, 2017

View reviewed changes

brendandburns force-pushed the arm branch 2 times, most recently from 45279a1 to 56c109d Compare April 3, 2017 23:11

tjprescott mentioned this pull request Apr 3, 2017

[ResourceManager] Major issues with Lock commands #2694

Closed

11 tasks

tjprescott reviewed Apr 3, 2017

View reviewed changes

brendandburns force-pushed the arm branch from 56c109d to 7e408a4 Compare April 5, 2017 03:51

tjprescott reviewed Apr 5, 2017

View reviewed changes

derekbekoe added the Resource Manager label Apr 5, 2017

Add docs for az lock update.

09aca4b

brendandburns force-pushed the arm branch from 7e408a4 to 09aca4b Compare April 5, 2017 23:08

tjprescott approved these changes Apr 5, 2017

View reviewed changes

tjprescott merged commit 95f33ff into Azure:master Apr 5, 2017

johanste pushed a commit to johanste/azure-cli that referenced this pull request Apr 7, 2017

Add docs for az lock update. (Azure#2702)

7b06ed8

mozehgir added the ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group label Aug 14, 2019

haroldrandom added cla-not-required Resource Manager labels Oct 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docs for az lock update. #2702

Add docs for az lock update. #2702

brendandburns commented Mar 31, 2017

brendandburns commented Mar 31, 2017

tjprescott left a comment

tjprescott Apr 3, 2017

brendandburns Apr 3, 2017

tjprescott Apr 3, 2017

brendandburns Apr 3, 2017

tjprescott Apr 3, 2017

tjprescott Apr 3, 2017

brendandburns Apr 3, 2017

tjprescott Apr 3, 2017

brendandburns Apr 3, 2017

brendandburns commented Apr 3, 2017

tjprescott Apr 3, 2017 •

edited

tjprescott Apr 3, 2017

brendandburns Apr 5, 2017

codecov-io commented Apr 5, 2017 •

edited

tjprescott Apr 5, 2017

brendandburns Apr 5, 2017

brendandburns commented Apr 5, 2017

Add docs for az lock update. #2702

Add docs for az lock update. #2702

Conversation

brendandburns commented Mar 31, 2017

brendandburns commented Mar 31, 2017

tjprescott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendandburns commented Apr 3, 2017

tjprescott Apr 3, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Apr 5, 2017 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendandburns commented Apr 5, 2017

tjprescott Apr 3, 2017 •

edited

codecov-io commented Apr 5, 2017 •

edited