containerapp - Add --source and --repo to containerapp create/update #6574

snehapar9 · 2023-07-28T16:17:02Z

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:

My extension description/summary conforms to the Extension Summary Guidelines.

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.

…to snehapar/Add-source-and-repo-to-create

…dd-source-to-containerapp-update

azure-client-tools-bot-prd · 2023-07-28T16:17:05Z

⚠️Azure CLI Extensions Breaking Change Test

⚠️containerapp

rule cmd_name rule_message suggest_message

⚠️ 1006 - ParaAdd containerapp create cmd containerapp create added parameter branch

⚠️ 1006 - ParaAdd containerapp create cmd containerapp create added parameter context_path

⚠️ 1006 - ParaAdd containerapp create cmd containerapp create added parameter repo

⚠️ 1006 - ParaAdd containerapp create cmd containerapp create added parameter service_principal_client_id

⚠️ 1006 - ParaAdd containerapp create cmd containerapp create added parameter service_principal_client_secret

⚠️ 1006 - ParaAdd containerapp create cmd containerapp create added parameter service_principal_tenant_id

⚠️ 1006 - ParaAdd containerapp create cmd containerapp create added parameter source

⚠️ 1006 - ParaAdd containerapp create cmd containerapp create added parameter token

⚠️ 1006 - ParaAdd containerapp update cmd containerapp update added parameter source

azure-client-tools-bot-prd · 2023-07-28T16:17:08Z

Hi @snehapar9,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

azure-client-tools-bot-prd · 2023-07-28T16:17:11Z

Hi @snehapar9,
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.

yonzhan · 2023-07-28T16:17:15Z

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

Greedygre · 2023-09-08T08:30:14Z

LGTM.
@Juliehzl Could you please have a look?

Juliehzl · 2023-09-08T10:31:18Z

src/containerapp/azext_containerapp/containerapp_decorator.py

+        self.set_up_source()
+
+    def set_up_source(self):
+        if self.get_argument_source():


does your feature only work for managed environment? If yes, please add validation for --environment-type.
For more information, @Greedygre can help.

This feature only work for managed environment. Because it use a lot of class from up, which only support managed environment.
@snehapar9
please add validation for --environment-type, if environement-type value is connected, or managed_env value is a connected environment reource id, should not use --repo and --source.

Juliehzl · 2023-09-08T10:33:27Z

src/containerapp/azext_containerapp/custom.py

                        no_wait=False,
-                        secret_volume_mount=None):
+                        secret_volume_mount=None,
+                        source=None):


In this PR, you also want to support --source for update command?

yes, we are adding support for --source as well for update in this PR.

Juliehzl · 2023-09-08T10:33:59Z

pls update history.rst for release notes

Juliehzl · 2023-09-11T02:39:13Z

src/containerapp/azext_containerapp/tests/latest/utils.py

+            JMESPathCheck('properties.provisioningState', 'Succeeded'),
+        ]).get_output_in_json()
+        test_cls.assertEqual(app["properties"]["template"]["containers"][0]["image"].split(":")[0], image_name)
+        test_cls.assertNotEqual(app["properties"]["template"]["containers"][0]["image"], old_image)


add container app delete and env delete commands to make sure all resources deleted

Greedygre · 2023-09-11T03:08:11Z

Please resolve zunli's latest comment, otherwise LGTM.

snehapar9 · 2023-09-11T23:13:47Z

Please resolve zunli's latest comment, otherwise LGTM.

Committed test changes to delete containerapp and environment at the end of the tests.

Greedygre · 2023-09-12T05:11:06Z

Hi @zhoxing-ms
This PR has been reviewed by me and zunli, could you help to review and merge? Thanks.

zhoxing-ms · 2023-09-12T08:28:15Z

src/containerapp/azext_containerapp/tests/latest/test_containerapp_create_update_with_source.py

+TEST_DIR = os.path.abspath(os.path.join(os.path.abspath(__file__), '..'))
+
+class ContainerAppCreateTest(ScenarioTest):
+    @live_only()


May I ask why do you mark these tests as @live_only()

These tests require a docker push operation to push the image built as part of the test to the container registry and would not execute from the CI pipeline since docker is not installed in the CI.

OK, please add comments in the test to explain this reason

zhoxing-ms · 2023-09-13T01:49:49Z

src/containerapp/azext_containerapp/_validators.py

+        parsed = urlparse(registry_server)
+        registry_name = (parsed.netloc if parsed.scheme else parsed.path).split(".")[0]
+        if registry_name and len(registry_name) > MAXIMUM_SECRET_LENGTH:
+            raise ValidationError(f"--registry-server ACR name must be less than {MAXIMUM_SECRET_LENGTH} "


Suggested change

raise ValidationError(f"--registry-server ACR name must be less than {MAXIMUM_SECRET_LENGTH} "

raise InvalidArgumentValueError(f"--registry-server ACR name must be less than {MAXIMUM_SECRET_LENGTH} "

Actually, ValidationError is acceptable, but personally, I think InvalidArgumentValueError is a more accurate usage.
If you want to maintain consistency with _validate_up_args, then I think it's also OK. Or modify them all to InvalidArgumentValueError

Thank you for taking a look! Just going to keep it as-is so we are consistent with this validation in the up command.

snehapar9 and others added 24 commits June 30, 2023 08:43

Add --source and --repo to containerapp create

887ecf6

Refactored code for containerapp create

011439a

Merge branch 'main' into snehapar/Add-source-and-repo-to-create

e9fd59b

Merge branch 'snehapar/add-source-and-repo-to-containerapp-create' in…

89241ca

…to snehapar/Add-source-and-repo-to-create

Removed extra source param

916259c

Addressed PR comments

1439120

Removed new line

fb7f4fe

Fixed linting errors

d18d6c7

Remove new line

78181aa

Minor fixes for GitHub repo params

d646bd1

Addressed PR comments

56e68d4

Merge branch 'main' into snehapar/Add-source-and-repo-to-create

1b70cf5

Add source back

3a809ca

Remove return statement

07e1be5

Addressed PR comments

25862fb

Add source flag to update

5845255

Refactor validation

0b2098b

Revert validation change

3fd9fde

Add live_only to tests

557f05e

Refactored update

6c3a58e

Merge branch 'snehapar/Add-source-and-repo-to-create' into snehapar/A…

d1631be

…dd-source-to-containerapp-update

Add tests for update

ab167d0

Update integration test

2af684f

Fix help text

a1afa52

azure-client-tools-bot-prd bot added this to the August 2023 (2023-09-05) milestone Jul 28, 2023

microsoft-github-policy-service bot requested a review from yonzhan July 28, 2023 16:17

Juliehzl reviewed Sep 8, 2023

View reviewed changes

snehapar9 added 3 commits September 8, 2023 13:46

Addressed PR comments

f1ac303

Addressed PR comments and performed refactoring

648090d

Fix linting

3aaea4f

snehapar9 marked this pull request as draft September 11, 2023 01:57

snehapar9 marked this pull request as ready for review September 11, 2023 02:10

Juliehzl reviewed Sep 11, 2023

View reviewed changes

Greedygre approved these changes Sep 11, 2023

View reviewed changes

snehapar9 added 3 commits September 11, 2023 09:25

Add env deletion at the end of test

1200a8c

Re-run test

a2888b1

Address PR comments and re-run tests

85c4302

snehapar9 added 2 commits September 11, 2023 17:27

Refactor and re-run tests

47f563c

Merge branch 'main' into snehapar/Add-source-to-containerapp-update

ab09b8c

Greedygre approved these changes Sep 12, 2023

View reviewed changes

Juliehzl approved these changes Sep 12, 2023

View reviewed changes

zhoxing-ms reviewed Sep 12, 2023

View reviewed changes

Merge branch 'main' into snehapar/Add-source-to-containerapp-update

b77faac

zhoxing-ms reviewed Sep 13, 2023

View reviewed changes

Added comment for live only tests

04a738c

zhoxing-ms approved these changes Sep 13, 2023

View reviewed changes

Add ACR registry validation

0badf02

Greedygre approved these changes Sep 13, 2023

View reviewed changes

zhoxing-ms merged commit 2aad26b into Azure:main Sep 13, 2023

snehapar9 mentioned this pull request Sep 13, 2023

containerapp - Add --source and --repo to containerapp create #6463

Closed

3 tasks

	raise ValidationError(f"--registry-server ACR name must be less than {MAXIMUM_SECRET_LENGTH} "
	raise InvalidArgumentValueError(f"--registry-server ACR name must be less than {MAXIMUM_SECRET_LENGTH} "

containerapp - Add --source and --repo to containerapp create/update #6574

containerapp - Add --source and --repo to containerapp create/update #6574

Uh oh!

Conversation

snehapar9 commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related command

General Guidelines

About Extension Publish

Uh oh!

azure-client-tools-bot-prd bot commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

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

Uh oh!

yonzhan commented Jul 28, 2023

Uh oh!

Greedygre commented Sep 8, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Greedygre Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Juliehzl commented Sep 8, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Greedygre commented Sep 11, 2023

Uh oh!

snehapar9 commented Sep 11, 2023

Uh oh!

Greedygre commented Sep 12, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

snehapar9 commented Jul 28, 2023 •

edited

Loading

azure-client-tools-bot-prd bot commented Jul 28, 2023 •

edited

Loading

Greedygre Sep 8, 2023 •

edited

Loading