Skip to content

Conversation

@adewaleo
Copy link
Contributor

Description

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


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

@yungezz
Copy link
Member

yungezz commented Nov 19, 2020

hi @fengzhou-msft could you pls help to review?

Copy link
Member

@yungezz yungezz left a comment

Choose a reason for hiding this comment

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

pls not checkin binary file

Copy link
Member

Choose a reason for hiding this comment

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

pls help to make sure run full live test for regression. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have re-run tests, please can you review @yungezz

@adewaleo
Copy link
Contributor Author

@yungezz thanks for the heads up and review. This is a wip pr with the generated sdk. I will remove it when the sdk has been published.

I will also make sure to run a full live test locally to ensure no regressions with the new api version.

@adewaleo adewaleo changed the title [ACR] bump api version. Use draft sdk. support zone redundancy [ACR] bump api version. support zone redundancy and network bypass Dec 17, 2020
@adewaleo adewaleo force-pushed the tosin/zone-redundancy branch 3 times, most recently from 5e76392 to 7ad595e Compare December 17, 2020 19:31
@adewaleo
Copy link
Contributor Author

@yungezz @fengzhou-msft please can your review? i am hoping to make next week's code complete. Moreover some acr tests are broken because networking command loading / parameter registration is having issues. I have latest dev.

This section in network/_params.py is causing problems:

        # region ApplicationGateways
        with self.argument_context('network application-gateway') as c:
            c.argument('application_gateway_name', app_gateway_name_type, options_list=['--name', '-n'])
>           c.argument('sku', arg_group='Gateway', help='The name of the SKU.', arg_type=get_enum_type(ApplicationGatewaySkuName), default=ApplicationGatewaySkuName.standard_medium.value)
E           AttributeError: 'NoneType' object has no attribute 'standard_medium'

this happens when tests like the one here try to create a vnet:

During handling of the above exception, another exception occurred:

self = <azure.cli.command_modules.acr.tests.latest.test_acr_commands.AcrCommandsTests testMethod=test_acr_with_private_endpoint>, resource_group = 'clitest.rg000001'

    @ResourceGroupPreparer(location='centraluseuap')
    def test_acr_with_private_endpoint(self, resource_group):
        self.kwargs.update({
            'registry_name': self.create_random_name('testreg', 20),
            'vnet_name': self.create_random_name('testvnet', 20),
            'subnet_name': self.create_random_name('testsubnet', 20),
            'endpoint_name': self.create_random_name('priv_endpoint', 25),
            'endpoint_conn_name': self.create_random_name('priv_endpointconn', 25),
            'second_endpoint_name': self.create_random_name('priv_endpoint', 25),
            'second_endpoint_conn_name': self.create_random_name('priv_endpointconn', 25),
            'description_msg': 'somedescription'
        })

        # create subnet with disabled endpoint network policies
>       self.cmd('network vnet create -g {rg} -n {vnet_name} --subnet-name {subnet_name}')

src\azure-cli\azure\cli\command_modules\acr\tests\latest\test_acr_commands.py:537:

@yonzhan
Copy link
Collaborator

yonzhan commented Dec 17, 2020

@jsntcy for awareness.

@jsntcy
Copy link
Member

jsntcy commented Dec 18, 2020

@jsntcy Yu Chen FTE for awareness.

As we're migrating network to track2 SDK, hopefully the migration can be done by the end of this week and will check if this issue still exists after migration.

@yungezz
Copy link
Member

yungezz commented Dec 18, 2020

LGTM

@yungezz
Copy link
Member

yungezz commented Dec 18, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@adewaleo adewaleo changed the title [ACR] bump api version. support zone redundancy and network bypass [ACR] bump api version. support zone redundancy ~~and network bypass~~ Dec 22, 2020
@adewaleo adewaleo changed the title [ACR] bump api version. support zone redundancy ~~and network bypass~~ [ACR] bump api version. support zone redundancy Dec 22, 2020
@adewaleo
Copy link
Contributor Author

@yungezz please can you reapprove and merge when ready. I am no longer adding the --bypass option and tests and styliing should be good too.

@adewaleo adewaleo force-pushed the tosin/zone-redundancy branch from 955aa71 to dca81fd Compare December 23, 2020 06:08
@yonzhan yonzhan merged commit 3776643 into Azure:dev Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants