Skip to content

Conversation

@warren-jones
Copy link
Contributor

@warren-jones warren-jones commented Jul 19, 2022

Currently, almost all quantum CLI command parameters in the az quantum CLI reference documentation appear under “Optional Parameters”, which gives the impression that none of these parameters are required parameters. This has been criticized as confusing and misleading.

The az quantum CLI Reference documentation is automatically generated from source files like _params.py, _help.py, and the command function signatures, so it is not directly editable like our tutorials and how-to guides. The source code dictates which parameters are documented as required, and which parameters are optional.

A positive side effect of making these code changes is a dramatic improvement in the detail level of error messages about missing parameters, since we're leveraging the framework's error message generation feature that adds help examples.

The required vs. optional treatment of --location and --resource-group is not consistent throughout the Azure CLI, but for example, Azure Virtual Machines generally treats --resource-group as a required parameter, see https://docs.microsoft.com/en-us/cli/azure/vm/aem?view=azure-cli-latest

I believe the least user confusion will arise from designating them Required Parameters to leverage the CLI framework’s validation and detailed error message generation. For --location, the help text always includes You can configure the default location using az configure --defaults location=<location> and a similar note appears for --resource-group.


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

Related command

All az quantum commands

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

@ghost ghost added the Auto-Assign Auto assign by bot label Jul 19, 2022
@ghost ghost requested review from kairu-ms and necusjz July 19, 2022 21:23
@ghost ghost assigned kairu-ms Jul 19, 2022
@ghost ghost added this to the Jul 2022 (2022-08-02) milestone Jul 19, 2022
@ghost ghost requested a review from yonzhan July 19, 2022 21:23
@ghost ghost added the Quantum az quantum label Jul 19, 2022
@ghost ghost requested a review from wangzelin007 July 19, 2022 21:23
@yonzhan
Copy link
Collaborator

yonzhan commented Jul 19, 2022

Quantum

@warren-jones
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 5130 in repo Azure/azure-cli-extensions

@ricardo-espinoza
Copy link
Contributor

Overall looks good to me, but we need to verify that the expectations from users when setting a default workspace (and its resource group) have not changed. Many of the commands were written with the idea that users would not need to specify -w/-g/-l if they already did an azure quantum workspace set, just in the same way that a target could also be set.

If usage has changed as a result of this PR, we need to discuss it in more detail.

@warren-jones
Copy link
Contributor Author

@ricardo-espinoza,
I responded to your "Overall looks good to me, but we need to verify that the expectations from users..." comment in email, but I'll reiterate it here to keep the conversation together:

az quantum workspace set has the same effect as before -- so I think we’re OK. The difference is internal: Workspace name is saved in the [defaults] section of the .azure/config file instead or [quantum], along with group and location. Do you see a problem there?

@warren-jones warren-jones requested review from ricardo-espinoza and removed request for kairu-ms, necusjz, wangzelin007 and yonzhan October 5, 2022 00:42
@warren-jones warren-jones changed the title [Quantum] [DRAFT - Do not merge] Show required parameters in CLI Reference documentation [Quantum] Show required parameters in CLI Reference documentation Oct 7, 2022
@kairu-ms kairu-ms marked this pull request as ready for review October 7, 2022 06:03
@kairu-ms kairu-ms requested a review from shenyingjun October 7, 2022 06:04
@kairu-ms kairu-ms marked this pull request as draft October 7, 2022 06:06
@warren-jones warren-jones marked this pull request as ready for review October 7, 2022 18:03
Copy link
Contributor

@ricardo-espinoza ricardo-espinoza left a comment

Choose a reason for hiding this comment

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

Approved, but I hope that by the next release (after this one) the commented code is fixed.

@kairu-ms
Copy link
Contributor

kairu-ms commented Oct 8, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kairu-ms kairu-ms merged commit a9d7efa into Azure:main Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto-Assign Auto assign by bot Quantum az quantum

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants