Skip to content

Conversation

@toki95
Copy link
Contributor

@toki95 toki95 commented Aug 10, 2020

Description

Add az sql mi op list, az sql mi op get to display operation steps and operation parameters

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.

@yonzhan yonzhan added this to the S174 milestone Aug 10, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented Aug 10, 2020

SQL

@toki95
Copy link
Contributor Author

toki95 commented Aug 10, 2020

Old PR #14515 abandoned due to branch issues.


self.kwargs.update({
'subnet_id': subnet['id']
'subnet_id': '/subscriptions/a8c9a924-06c0-4bde-9788-e7b1370969e1/resourceGroups/v-urmila/providers/Microsoft.Network/virtualNetworks/MIVirtualNetwork/subnets/ManagedInsanceSubnet'
Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed resource is not recommend in azure cli because it is hard for others to re-run with such resources. May I know why you want to fix resources here?

@Juliehzl
Copy link
Contributor

Hi @toki95, from my investigation, the PR only upgrades package version but as you mentioned in PR title, it should also add new parameters in some existing commands. With the new package, new parameters will be added automatically or you still need additional work?

@Juliehzl
Copy link
Contributor

@toki95 any update?

@yonzhan yonzhan removed this from the S174 milestone Aug 22, 2020
@yonzhan yonzhan added this to the S175 - For Ignite milestone Aug 22, 2020
@toki95
Copy link
Contributor Author

toki95 commented Aug 24, 2020

@toki95 any update?

Sorry I was OOF, I don't need any additional work.
This is ready for merge.

@toki95
Copy link
Contributor Author

toki95 commented Aug 26, 2020

@Juliehzl can we merge this PR now?


self.kwargs.update({
'subnet_id': subnet['id']
'subnet_id': '/subscriptions/a8c9a924-06c0-4bde-9788-e7b1370969e1/resourceGroups/v-urmila/providers/Microsoft.Network/virtualNetworks/MIVirtualNetwork/subnets/ManagedInsanceSubnet'
Copy link
Contributor

Choose a reason for hiding this comment

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

fixed resource is not recommended in test.

Copy link
Contributor

Choose a reason for hiding this comment

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

The main issue with this is that MI tests are not easily setup. They also run very long when fully setup (>4h). I agree that it should be fixed. My take is that I believe Jovana is not making tests non-recordable, since they usually are, and with or without her change, people would need to do some manual setup/preparing in order to record.

Copy link
Contributor

Choose a reason for hiding this comment

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

discussed with Jovana and Perica offline. They have to make network resource fixed given policy set on their subscription.

is it possible to disable the policy before you run the test and enable it again after done?

Copy link
Contributor

Choose a reason for hiding this comment

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

discussed with Jovana. They cannot disable policy.

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.

I haven't find command parameter change in your description. Because PR title will be used as release note, please change the description for what you did in the PR.

From my perspective, what you do is just "upgrade azure-mgmt-sql to 0.20.0". IF it is not correct, feel free to use your words.

@toki95 toki95 changed the title [SQL] NEW Add az sql mi op list, az sql mi op get to display operation steps and operation parameters [SQL] upgrade azure-mgmt-sql to 0.20.0 Sep 1, 2020
@toki95
Copy link
Contributor Author

toki95 commented Sep 1, 2020

I haven't find command parameter change in your description. Because PR title will be used as release note, please change the description for what you did in the PR.

From my perspective, what you do is just "upgrade azure-mgmt-sql to 0.20.0". IF it is not correct, feel free to use your words.

Done

@Juliehzl Juliehzl merged commit 79d71de into Azure:dev Sep 3, 2020
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.

7 participants