Skip to content

Conversation

@qwordy
Copy link
Member

@qwordy qwordy commented Mar 2, 2021

Add a file service_name.json. It maintains service information of commands.

Twin PR, Azure/azure-cli#17159. Visit it for more details.

Example

  {
    "Command": "az databox",
    "Description": "Microsoft Azure Command-Line Tools DataBox Extension",
    "AzureServiceName": "Azure Databox",
    "ConversationalServiceName": "Databox",
    "URL": "https://docs.microsoft.com/azure/databox/"
  },

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

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?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

@yonzhan yonzhan requested a review from jiasli March 2, 2021 04:52
@@ -0,0 +1,422 @@
[
{
"Command": "az ai-did-you-mean-this",
Copy link
Member

Choose a reason for hiding this comment

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

I believe this extension is deprecated already, could you pls double confirm?

"URL": ""
},
{
"Command": "az ai-examples",
Copy link
Member

Choose a reason for hiding this comment

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

how to maintain this file? what's process when new extension is released?

Copy link
Member Author

Choose a reason for hiding this comment

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

Manual. We can add a linter rule to force it.
I added a new column "ExtensionName". It is required when "AzureCLIReferenceType" is Extension or "hasExtension" is 1. This column is used to check if all extensions have records.

@qwordy qwordy requested a review from fengzhou-msft March 9, 2021 06:56
"AzureServiceName": "Azure CLI",
"ConversationalServiceName": "Azure CLI",
"ExtensionName": "alias",
"URL": "https://docs.microsoft.com/cli/azure/"
Copy link
Member

Choose a reason for hiding this comment

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

Where does this URL come from? The current one is a general CLI doc link.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can set it reference of az alias

Comment on lines 490 to 497
{
"Command": "az timeseriesinsights",
"Description": "Microsoft Azure Command-Line Tools TimeSeriesInsightsClient Extension",
"AzureServiceName": "Azure Time Series Insights",
"ConversationalServiceName": "Time Series Insights",
"ExtensionName": "timeseriesinsights",
"URL": "https://docs.microsoft.com/azure/time-series-insights/"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This command group is deprecated. Now its name is "az tsi" which already included below.

Suggested change
{
"Command": "az timeseriesinsights",
"Description": "Microsoft Azure Command-Line Tools TimeSeriesInsightsClient Extension",
"AzureServiceName": "Azure Time Series Insights",
"ConversationalServiceName": "Time Series Insights",
"ExtensionName": "timeseriesinsights",
"URL": "https://docs.microsoft.com/azure/time-series-insights/"
},

Comment on lines 283 to 297
"Command": "az k8s-configuration",
"Description": "Manage Kubernetes configurations",
"AzureServiceName": "Azure Kubernetes",
"ConversationalServiceName": "Kubernetes Service",
"ExtensionName": "k8s-configuration",
"URL": "https://docs.microsoft.com/azure/aks/"
},
{
"Command": "az k8sconfiguration",
"Description": "Microsoft Azure Command-Line Tools K8sconfiguration Extension",
"AzureServiceName": "Azure Kubernetes Service",
"ConversationalServiceName": "Kubernetes Service",
"ExtensionName": "k8sconfiguration",
"URL": "https://docs.microsoft.com/azure/aks/"
},
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 k8s-configuration and k8sconfiguration are the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, they are two extension. Ugly naming.

k8s-configuration               1.0.0      Microsoft Azure Command-Line Tools K8s-configuration Extension                                                     False      False           False
k8sconfiguration                0.2.4      Microsoft Azure Command-Line Tools K8sconfiguration Extension                                                      True       False           False

{
"Command": "az confluent",
"Description": "Manage confluent resources.",
"AzureServiceName": "",
Copy link
Member

Choose a reason for hiding this comment

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

blank?

Copy link
Member Author

Choose a reason for hiding this comment

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

Have not found an appropriate value

},
{
"Command": "az alias",
"Description": "Support for command aliases",
Copy link
Contributor

@dbradish-microsoft dbradish-microsoft Mar 10, 2021

Choose a reason for hiding this comment

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

@yungezz , @qwordy , @jiasli , @fengzhou-msft

Important: Shuang is already keeping a new JSON file which replaces TitleMapping.json and where this information would fit perfectly. Ideally, we'd like to avoid having engineering enter autogenerated content in multiple places. Can we please check with Alice Wang and Shuang Jiang on a consolidated place for this information?

Other thoughts:

  1. I feel we should remove "Description" as it is a duplicate of the autogenerated content. There is no reason for making someone enter the same information in two different places, correct?
  2. Isn't "extensionName" also a duplicate from what is found in the _help.py or commands.py files? Shuang is currently showing this information at the top of every autogenerated extension page and he doesn't use this new file.
  3. URL is not necessary unless we want to a.) take responsibility for maintaining this link or b.) use it in our autogenerated content. I had it in SQL in order to find the Azure service home page for my reference (vs Googling it every time I needed it)
  4. I am glad to see "conversational service name" removed -- that was only for my Power BI internal reporting.

Copy link
Member Author

Choose a reason for hiding this comment

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

@qwordy
Copy link
Member Author

qwordy commented Mar 24, 2021

@dbradish-microsoft Can you revoke the "request change"? Thanks.

@qwordy
Copy link
Member Author

qwordy commented Mar 24, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@dbradish-microsoft dbradish-microsoft self-requested a review March 24, 2021 12:28
@dbradish-microsoft
Copy link
Contributor

@qwordy , done.

@qwordy
Copy link
Member Author

qwordy commented Mar 29, 2021

I moved content to these 2 PRs.
Azure/azure-cli#17428
#3186

@qwordy qwordy closed this Mar 29, 2021
@qwordy qwordy reopened this Apr 1, 2021
@qwordy
Copy link
Member Author

qwordy commented Apr 1, 2021

Reopen this PR. The following two new PRs that contain linter and service_name.json are still in review. Reviewing Azure CLI Core code is always hard. As a result, I decide to split linter and service_name.json into separate PRs. Let's merge the service_name.json PR first.
Azure/azure-cli#17428
#3186

@qwordy qwordy merged commit 3dd4217 into Azure:master Apr 1, 2021
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