Skip to content

Conversation

@Greedygre
Copy link
Contributor

@Greedygre Greedygre commented Jul 11, 2023


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

Refactor clients for separating GA and preview extension.
For containerapp extension, use CURRENT_API_VERSION to control version.
For containerapp-preview extension, use PREVIEW_API_VERSION. The command in containerapp-preview will overwrite the command in containerapp, and api-version value is PREVIEW_API_VERSION. #6419

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.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Jul 11, 2023

️✔️Azure CLI Extensions Breaking Change Test
️✔️Non Breaking Changes

@azure-client-tools-bot-prd
Copy link

Hi @Greedygre,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@ghost ghost requested a review from yonzhan July 11, 2023 14:50
@ghost ghost added the Auto-Assign Auto assign by bot label Jul 11, 2023
@yonzhan
Copy link
Collaborator

yonzhan commented Jul 11, 2023

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

@Greedygre Greedygre force-pushed the xinyu/refactor_client2 branch from b72ee3e to 00b7b8c Compare July 12, 2023 01:50
@Greedygre
Copy link
Contributor Author

Greedygre commented Jul 12, 2023

hi @zhoxing-ms
I found after the pr #6419 merged, the pipeline check failed due to:


======================================================================
ERROR: test_ref_doc_containerapp-preview (__main__.IndexRefDocs)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/vss/_work/1/s/./scripts/ci/index_ref_doc.py", line 66, in test
    raise e
  File "/mnt/vss/_work/1/s/./scripts/ci/index_ref_doc.py", line 63, 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/tmpz06ma0oi/containerapp_preview-1.0.0b1-py3-none-any.whl', '--output-dir', '/tmp/tmpogmsf8tt/containerapp-preview']' returned non-zero exit status 1.


The root error msg is as following. The containerapp-preview's command can't run independently. Need to install containerapp extension.


 Exception occurred:

File "/tmp/tmpqw7eb4dn/extension/azext_containerapp_preview/_utils.py", line 33, in _get_azext_containerapp_module

raise ValidationError(f"The command requires the version of {GA_CONTAINERAPP_EXTENSION_NAME} >= {MIN_GA_VERSION}. Run 'az extension add --upgrade -n {GA_CONTAINERAPP_EXTENSION_NAME}' to install extension")

azure.cli.core.azclierror.ValidationError: The command requires the version of containerapp >= 0.3.35. Run 'az extension add --upgrade -n containerapp' to install extension

@wangzelin007 wangzelin007 force-pushed the xinyu/refactor_client2 branch from 157858e to db99bfc Compare July 12, 2023 05:20
run(cmd, check=True)

def tearDown(self):
cmd = ['azdev', 'extension', 'remove', 'containerapp']
Copy link
Contributor Author

@Greedygre Greedygre Jul 12, 2023

Choose a reason for hiding this comment

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

To make concurrent execution of tests more stable.

Copy link
Contributor Author

@Greedygre Greedygre Jul 12, 2023

Choose a reason for hiding this comment

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

@wangzelin007

This CI test failed Pipelines - Run 20230712.36 logs (azure.com)

only python 3.10 tests failed. And Error as following.


[gw0] [ 50%] FAILED src/containerapp-preview/azext_containerapp_preview/tests/latest/test_containerapp_preview_scenario.py::ContainerappScenarioTest::test_containerapp_preview_e2e

src/containerapp-preview/azext_containerapp_preview/tests/latest/test_containerapp_preview_scenario.py::ContainerappScenarioTest::test_containerapp_preview_environment_type

[gw0] [100%] PASSED src/containerapp-preview/azext_containerapp_preview/tests/latest/test_containerapp_preview_scenario.py::ContainerappScenarioTest::test_containerapp_preview_environment_type

 

ERROR cli.azure.cli.core.azclierror:azlogging.py:212 unrecognized arguments: --environment-type managed

ERROR az_command_data_logger:azlogging.py:213 unrecognized arguments: --environment-type managed


Copy link
Contributor Author

Choose a reason for hiding this comment

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

And with more and more test cases( with different file), this kind of azdev extension add containerapp in a case will be unstable

Copy link
Contributor

@Juliehzl Juliehzl left a comment

Choose a reason for hiding this comment

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

approved with suggestions for tearDown

@Greedygre
Copy link
Contributor Author

Greedygre commented Jul 12, 2023

Hi @zhoxing-ms
Could you help to review and merge this pr?

BTW:
This tearDown() change has been confirmed with zelin to have no effect, because the pipeline test agent is destroyed when the test is all done, so not removing it has no effect.

If there is a better way to install dependency containerapp extension it will be in a subsequent iteration.

@zhoxing-ms zhoxing-ms merged commit 1ddb047 into Azure:main Jul 12, 2023
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.

5 participants