Skip to content

Conversation

@arrownj
Copy link
Contributor

@arrownj arrownj commented Apr 30, 2020

Description
This PR is to support below features for local context:

  • change local context to global user level
  • divide different default value source in help message
  • add warning message when local context is on and take effect in current command
  • add show/delete command for az local-context
  • add LocalContextScenarioTest class in testsdk. All test cases for local context should extend it and it will do turn on/off work automatically.

Testing Guide
az local-context on

Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
Local context is turned on, you can run az local-context off to turn it off.

az group create -g myGroup -l eastasia

Local context is turned on. Its information is saved in working directory D:\code\cli. You can run az local-context off to turn it off.
Command argument values saved to local context: --name: xxj1
{
"id": "/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/resourceGroups/myGroup ",
"location": "eastasia",
"managedBy": null,
"name": "myGroup ",
"properties": {
"provisioningState": "Succeeded"
},
"tags": {
"StorageType": "Standard_LRS",
"type": "test"
},
"type": "Microsoft.Resources/resourceGroups"
}

az network vnet create -n myVNet --subnet-name mySubnet

Local context is turned on. Its information is saved in working directory D:\code\cli. You can run az local-context off to turn it off.
Command argument values from local context: --resource-group: xxj1
Local context is turned on. Its information is saved in working directory D:\code\cli. You can run az local-context off to turn it off.
Command argument values saved to local context: --name: myVNet, --subnet-name: mySubnet
{
"newVNet": {
"addressSpace": {
"addressPrefixes": [
"10.0.0.0/16"
]
},
...
}

az local-context show

Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
{
"all": {
"resource_group_name": "myGroup ",
"subnet_name": "mySubnet",
"vnet_name": "myVNet"
}
}

az local-context show --name vnet_name subnet_name

Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
{
"all": {
"subnet_name": "mySubnet",
"vnet_name": "myVNet"
}
}

az local-context delete --name resource_group_name

Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
Local context value is deleted. You can run az local-context show to show all available values.

az local-context show

Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
{
"all": {
"subnet_name": "mySubnet",
"vnet_name": "myVNet"
}
}

az local-context delete --all

Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
You are going to clear all local context value. Are you sure you want to continue this operation ? (y/n): y
Local context information in working directory D:\code\cli is cleared.

az local-context delete --all --purge

Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
You are going to delete local context persistence file. Are you sure you want to continue this operation ? (y/n): y
Local context persistence file in working directory D:\code\cli is deleted.

az local-context off

Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
Local context is turned off, you can run az local-context on to turn it on.

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


This checklist is used to make sure that common guidelines for a pull request are followed.

@yonzhan
Copy link
Collaborator

yonzhan commented Apr 30, 2020

local context

@arrownj arrownj requested a review from bim-msft as a code owner May 8, 2020 05:19
@arrownj arrownj changed the title [WIP][Core] update local context to global user level [Core] update local context to global user level May 8, 2020
@jiasli
Copy link
Member

jiasli commented May 11, 2020

I would prefer title

[Core] Update local context on/off status to global user level

self.initialize()

def initialize(self):
self.username = _get_current_username(self.cli_ctx)
Copy link
Member

Choose a reason for hiding this comment

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

_get_current_username [](start = 24, length = 21)

what about sp and msi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed in the review meeting, username here changed to system account.


def _load_local_context_file(self):
current_dir = os.getcwd()
current_dir = self.current_dir
Copy link
Member

Choose a reason for hiding this comment

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

current_dir = self.current_dir [](start = 8, length = 30)

do you need a local current_dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you can see, os.getcwd may throw an exception and the value may be frequently used in get/set function.

The benefits to add a local current_dir may contain these:

  1. control to print working directory warning only once easily. (If set/_load_ call os.getcwd directly, it may be a little complex to make it print the warning message only once because set/_load may be called multi times in one command)
  2. may have very little performance advantages.

logger.debug('The working directory has been deleted or recreated. Local context is ignored.')

self.local_context_file = None
if self.is_on and self.current_dir:
Copy link
Member

Choose a reason for hiding this comment

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

move to _load_local_context_file()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some logic updates here. _load_local_context_file update to _load_local_context_files, it may also be called if local context is off (show/delete command when local context is off). Also update self.local_context_file to self._local_context_file, it is loaded only once(when local context is on) and only used for get/set for local context parameter value.

@arrownj arrownj changed the title [Core] update local context to global user level [Core] Update local context on/off to global user level May 11, 2020
@arrownj arrownj changed the title [Core] Update local context on/off to global user level [Core] Update local context on/off status to global user level May 13, 2020
Copy link
Member

@qianwens qianwens left a comment

Choose a reason for hiding this comment

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

:shipit:

@arrownj arrownj merged commit 0ce0448 into Azure:dev May 13, 2020
@arrownj arrownj deleted the refact_local_context branch July 29, 2020 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants