Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Component Discovery #1409

Merged
merged 1 commit into from
Nov 22, 2016
Merged

Component Discovery #1409

merged 1 commit into from
Nov 22, 2016

Conversation

derekbekoe
Copy link
Member

‘az component list-available’ command
Check if the module is available if parsing fails due to command_package error

Resolves #863

@derekbekoe
Component Discovery
2e766b5 
‘az component list-available’ command
Check if the module is available if parsing fails due to command_package error
@mention-bot
Copy link

@derekbekoe, thanks for your PR! By analyzing the history of the files in this pull request, we identified @johanste, @tjprescott and @BurtBiel to be potential reviewers.

@derekbekoe
Copy link
Member Author 

$ az component list-available -h

Command
    az component list-available: List publicly available components that can be installed.

Arguments

There are extra modules that are available:

$ az component list-available
[
  {
    "name": "redis",
    "summary": "Microsoft Azure Command-Line Tools Redis Command Module",
    "version": "0.1.0b9"
  },
  {
    "name": "keyvault",
    "summary": "Microsoft Azure Command-Line Tools Keyvault Command Module",
    "version": "0.1.0b9"
  }
]

If you already have all the available components:

$ az component list-available
All available components are already installed.

Attempting to run a command for a component you don't have installed:

$ az redis
Install the 'redis' module with 'az component update --add redis'
usage: az [-h] [--output {json,tsv,list,table,jsonc}] [--verbose] [--debug]
          [--query JMESPATH]
          {network,resource,vmss,storage,tag,iot,vm,component,acs,keyvault,appservice,context,role,cloud,ad,acr,provider,feedback,account,taskhelp,container,logout,configure,login}
          ...
az: error: argument _command_package: invalid choice: 'redis' (choose from 'network', 'resource', 'vmss', 'storage', 'tag', 'iot', 'vm', 'component', 'acs', 'keyvault', 'appservice', 'context', 'role', 'cloud', 'ad', 'acr', 'provider', 'feedback', 'account', 'taskhelp', 'container', 'logout', 'configure', 'login')

tjprescott
tjprescott reviewed Nov 22, 2016
View reviewed changes
Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

One small question, but overall LGTM.

src/azure-cli-core/azure/cli/core/_pkg_util.py
import xmlrpc.client as xmlrpclib #pylint: disable=import-error
query = {'author': 'Microsoft Corporation', 'author_email': 'azpycli'}
logger.debug("Checking PyPI for modules using query '%s'", query)
client = xmlrpclib.ServerProxy('https://pypi.python.org/pypi')
Copy link
Member

Choose a reason for hiding this comment

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

Does this handle a private server? Does it need to?

Copy link
Member Author

Choose a reason for hiding this comment

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

It lists the modules from the public server.
This is because:

  1. There is no way when you run a command like az redis that you'd want it to look in private server as well. For component commands, users explicitly set --private.
  2. For the az component list-available command, it supports Public only because private servers don't have an API like the public PyPI to get available packages. Would have to scrape HTML pages to get these.

yugangw-msft
yugangw-msft reviewed Nov 22, 2016
View reviewed changes
src/azure-cli-core/azure/cli/core/parser.py
try:
possible_module = re.search("argument _command_package: invalid choice: '(.+?)'",
err_msg).group(1)
handle_module_not_installed(possible_module)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we only invoke module discovery for top level commands? say az foo, or we do it at deeper level across, say az resource group foo-bar.
I hope we only do it for top level unless the discovery takes trivial time.

Copy link
Contributor

Choose a reason for hiding this comment

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

i think we are good, because the code detects the _command_package wording

@yugangw-msft
Copy link
Contributor

LGTM

@derekbekoe
Copy link
Member Author

@johanste, @mayurid, @JasonRShaver please take a look at the PR comment (#1409 (comment)) and let me know if you're good with the experience.

@derekbekoe derekbekoe merged commit 4354da4 into Azure:master Nov 22, 2016
@derekbekoe derekbekoe deleted the component-discovery branch November 22, 2016 19:17
thegalah pushed a commit to thegalah/azure-cli that referenced this pull request Nov 28, 2016
Merge remote-tracking branch 'Azure/master'
7495297 
* Azure/master: (39 commits)
  User should use no-cache so we build a fresh image (Azure#1455)
  Bump all modules to version 0.1.0b10 (Azure#1454)
  [Docs] Move around the order of install instructions. (Azure#1439)
  acs: *_install_cli mark cli as executable (Azure#1449)
  Fix resource list table. (Azure#1452)
  [Compute] VM NIC updates (Azure#1421)
  Introduce batch upload and download for blob (Azure#1428)
  Add auto-registration for resource providers. (Azure#1330)
  interpret the '@' syntax if something higher hasn't already done that. (Azure#1423)
  Aliasing plan argument with shorthand (Azure#1433)
  ad:fix one more place which still uses localtime for secret starttime (Azure#1429)
  Add table formatting for deployments and sort by timestmap. (Azure#1427)
  Add table formatting for resource group list (and add 'status') (Azure#1425)
  [Docs] Add shields specifying latest version and supported python versions (Azure#1420)
  Add new az storage blob copy start-batch command (Azure#1250)
  Component Discovery (Azure#1409)
  Add poison logic to prevent re-recording tests that need updating. (Azure#1412)
  [Storage] Fix storage table outputs and help text. (Azure#1402)
  [mention-bot] Attempt to fix config (Azure#1410)
  ad:use utc time on setting app's creds (Azure#1408)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yugangw-msft yugangw-msft yugangw-msft left review comments

@tjprescott tjprescott tjprescott approved these changes

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@derekbekoe @mention-bot @yugangw-msft @tjprescott @azurecla @haroldrandom