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.

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

[Nginx] Add NGINX CLI #5305

Merged
merged 37 commits into from
Sep 27, 2022
Merged

[Nginx] Add NGINX CLI #5305

merged 37 commits into from
Sep 27, 2022

Conversation

limingu
Copy link
Member

@limingu limingu commented Sep 2, 2022


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

Related command

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 pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

@yonzhan
Copy link
Collaborator

yonzhan commented Sep 2, 2022

Nginx

@limingu limingu changed the title [Nginx] Add NGINX cli [Nginx] Add NGINX CLI Sep 14, 2022
Copy link
Contributor

@kairu-ms kairu-ms left a comment

Choose a reason for hiding this comment

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

Please add examples for commands

src/nginx/azext_nginx/azext_metadata.json Outdated Show resolved Hide resolved
policy.json Outdated Show resolved Hide resolved
src/nginx/azext_nginx/_help.py Outdated Show resolved Hide resolved
src/nginx/azext_nginx/_params.py Outdated Show resolved Hide resolved
src/nginx/setup.py Outdated Show resolved Hide resolved
@@ -0,0 +1,4 @@
{
"azext.isExperimental": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to release this extension in experimental, preview or GA?

Copy link
Contributor

@george-ngugi george-ngugi Sep 19, 2022

Choose a reason for hiding this comment

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

GA. Changing this to false

Comment on lines 9 to 19
helps['nginx deployment list'] = """
type: command
short-summary: "List all Nginx Deployments under the specified resource group; List all deployments under the specified subscription."
examples:
- name: Deployments_ListByResourceGroup
text: |-
az nginx deployment list --resource-group myResourceGroup
- name: Deployments_List
text: |-
az nginx deployment list
"""
Copy link
Contributor

@kairu-ms kairu-ms Sep 20, 2022

Choose a reason for hiding this comment

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

Because you use aaz-dev-tools to generate atomic commands. You should add examples and summaries in aaz-dev-tools. When you generate code, that information will be contained in the Command class description. Instead of writing them in _help.py file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please create a PR to submit the atomic command models in aaz as well.

Created PR Azure/aaz#24

Copy link
Contributor

Choose a reason for hiding this comment

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

Because you use aaz-dev-tools to generate atomic commands. You should add examples and summaries in aaz-dev-tools. When you generate code, that information will be contained in the Command class description. Instead of writing them in _help.py file.

Oh. Thanks for this. Done

@kairu-ms
Copy link
Contributor

Please create a PR to submit the atomic command models in aaz as well.

Comment on lines 20 to 26
parameters:
- name: --certificate-path
This path must match one or more ssl_certificate_key directive file argument in your Nginx configuration. This path must be unique between certificates within the same deployment
- name: --key-path
This path must match one or more ssl_certificate directive file argument in your Nginx configuration. This path must be unique between certificates within the same deployment
- name: --key-vault-secret-id
The secret ID for your certificate from Azure Key Vault
Copy link
Contributor

Choose a reason for hiding this comment

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

Parameter help message should be in the argument short-summery or long-summery in aaz-dev-tools.

Copy link
Contributor

@george-ngugi george-ngugi Sep 21, 2022

Choose a reason for hiding this comment

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

Done. I've also updated this on the aaz repo PR

@wangzelin007
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@george-ngugi
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

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

@kairu-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kairu-ms kairu-ms merged commit f947976 into Azure:main Sep 27, 2022
@azclibot
Copy link
Collaborator

[Release] Update index.json for extension [ nginx ] : https://dev.azure.com/azure-sdk/internal/_build/results?buildId=6256&view=results

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.

6 participants