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

containerapp add containerapp-preview extension #6419

Merged
merged 54 commits into from
Jul 12, 2023

Conversation

Greedygre
Copy link
Contributor

@Greedygre Greedygre commented Jun 19, 2023


We will have 2 extension: containerapp and containerapp-preview.
Generally, in containerapp extension we use stable api-version, in containerapp-preview extension we could use preview api-version.
Preview extension can be used to overwrite the command in the GA extension, which give us the capability to put the stable-only feature in the GA extension and use the preview extension for the preview-only feature.
The containerapp-preview extension cannot run independently, it requires the containerapp extension.
Here is the discussion:Azure/azure-cli#25782

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.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

@ghost ghost requested a review from yonzhan June 19, 2023 08:57
@ghost ghost added the Auto-Assign Auto assign by bot label Jun 19, 2023
@ghost ghost requested a review from wangzelin007 June 19, 2023 08:57
@yonzhan
Copy link
Collaborator

yonzhan commented Jun 19, 2023

Thank you for your contribution! We will review the pull request and get back to you soon.

@ghost ghost requested a review from yanzhudd June 19, 2023 08:57
@ghost ghost assigned zhoxing-ms Jun 19, 2023
@ghost ghost added the ContainerApp label Jun 19, 2023
@ghost ghost requested review from zhoxing-ms and jsntcy June 19, 2023 08:57
@Greedygre
Copy link
Contributor Author

wait #6397

@Juliehzl
Copy link
Contributor

remove empty file and make CI pass

@Greedygre Greedygre force-pushed the xinyu/add_create_containerapp_preview branch 2 times, most recently from 86dd794 to aac0694 Compare July 3, 2023 01:46
@Greedygre Greedygre closed this Jul 3, 2023
@Greedygre Greedygre reopened this Jul 3, 2023
@Greedygre Greedygre changed the title containerapp add create containerapp preview containerapp add containerapp-preview extension Jul 3, 2023
@Greedygre
Copy link
Contributor Author

Greedygre commented Jul 3, 2023

Hi @wangzelin007

The CI check [Azure.azure-cli-extensions (CLI Linter on Modified Extensions)]failed due to our containerapp-preview rely on containerapp extension.
When running command,
azdev linter --include-whl-extensions containerapp-preview
It will throw error:


"az containerapp auth apple show --name MyContainerapp --resource-group MyResourceGroup" is not a valid command.
"az containerapp auth facebook show --name MyContainerapp --resource-group MyResourceGroup" is not a valid command.

We should install containerapp and containerapp-preview both to pass the check.

And I don't know why the [Verify Ref Docs] failed too. Is there any problem in test pipeline?


ERROR: test_ref_doc_partnercenter (main.IndexRefDocs)
Traceback (most recent call last):
File "/mnt/vss/_work/1/s/./scripts/ci/index_ref_doc.py", line 65, in test
raise e
File "/mnt/vss/_work/1/s/./scripts/ci/index_ref_doc.py", line 62, in test
check_call(script_args)
File "/opt/hostedtoolcache/Python/3.10.12/x64/lib/python3.10/subprocess.py", line 369, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/opt/hostedtoolcache/Python/3.10.12/x64/bin/python', '/mnt/vss/_work/1/s/scripts/refdoc/generate.py', '--extension-file', '/tmp/tmpj5ap7dr4/partnercenter-0.2.3-py3-none-any.whl', '--output-dir', '/tmp/tmp05876jq9/partnercenter']' returned non-zero exit status 1.

Thanks a lot!

@wangzelin007
Copy link
Member

Hi @wangzelin007

The CI check [Azure.azure-cli-extensions (CLI Linter on Modified Extensions)]failed due to our containerapp-preview rely on containerapp extension. When running command, azdev linter --include-whl-extensions containerapp-preview It will throw error:


"az containerapp auth apple show --name MyContainerapp --resource-group MyResourceGroup" is not a valid command.
"az containerapp auth facebook show --name MyContainerapp --resource-group MyResourceGroup" is not a valid command.

We should install containerapp and containerapp-preview both to pass the check.

And I don't know why the [Verify Ref Docs] failed too. Is there any problem in test pipeline?


ERROR: test_ref_doc_partnercenter (main.IndexRefDocs)
Traceback (most recent call last):
File "/mnt/vss/_work/1/s/./scripts/ci/index_ref_doc.py", line 65, in test
raise e
File "/mnt/vss/_work/1/s/./scripts/ci/index_ref_doc.py", line 62, in test
check_call(script_args)
File "/opt/hostedtoolcache/Python/3.10.12/x64/lib/python3.10/subprocess.py", line 369, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/opt/hostedtoolcache/Python/3.10.12/x64/bin/python', '/mnt/vss/_work/1/s/scripts/refdoc/generate.py', '--extension-file', '/tmp/tmpj5ap7dr4/partnercenter-0.2.3-py3-none-any.whl', '--output-dir', '/tmp/tmp05876jq9/partnercenter']' returned non-zero exit status 1.

Thanks a lot!

  1. Add custom logic in azdev linter to support containerapp-preview: scripts/ci/verify_linter.py
  2. Fix by this PR

@wangzelin007
Copy link
Member

Hi @wangzelin007 The [Azure.azure-cli-extensions Breaking Change Test ] failed, could you help to handle it?Thanks.

update: Because the breaking change test rely on HEAD code to install extension, and the containerapp-preview is a new extension, so it failed to install for test in HEAD code. Won't block merged.

diff_ref = diff_code(src_branch, 'HEAD')

installing extension: containerapp-preview
cmd: ['azdev', 'extension', 'add', 'containerapp-preview']
ERROR: extension(s) not found: containerapp-preview

This is a warning level detection, I did not judge the extension of the first onboarding, but it will not affect pr merge.

@Greedygre Greedygre force-pushed the xinyu/add_create_containerapp_preview branch from 5922333 to 03aa729 Compare July 11, 2023 04:42
@Greedygre
Copy link
Contributor Author

Hi @zhoxing-ms

This pr is ready to merge, could you please help to merge? Thanks a lot.

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Jul 11, 2023

I have a small suggestion: perhaps in the future versions (not necessary for the current version), we could consider using config (az conig) to support enabling silent installation for containerapp extension with customer consent.

@Greedygre Greedygre mentioned this pull request Jul 11, 2023
3 tasks
@zhoxing-ms zhoxing-ms merged commit 6f6c5db into Azure:main Jul 12, 2023
14 checks passed
@azclibot
Copy link
Collaborator

[Release] Update index.json for extension [ containerapp-preview ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=71512&view=results

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 ContainerApp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants