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

{PolicyInsights} Update SDK package azure-mgmt-policyinsights to 1.1.0b2 #22043

Merged
merged 10 commits into from
May 13, 2022

Conversation

najagasi
Copy link
Contributor

@najagasi najagasi commented Apr 13, 2022

Description
This PR upgrades the azure-mgmt-policyinsights library to version 1.1.0b2 from 1.0.0
Testing Guide

History Notes

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

@yonzhan
Copy link
Collaborator

yonzhan commented Apr 13, 2022

AzurePolicy

@yonzhan yonzhan requested a review from zhoxing-ms April 13, 2022 01:02
@yonzhan yonzhan added this to the Apr 2022 (2022-04-26) milestone Apr 13, 2022
@yonzhan yonzhan requested review from wangzelin007 and jsntcy April 13, 2022 01:03
@wangzelin007
Copy link
Member

@najagasi,
Please rerun the following tests:
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation_complete
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation_management_group
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation_policy_set
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation_reevaluate

@zhoxing-ms
Copy link
Contributor

@najagasi Any update?

azure-mgmt-policyinsights==1.0.0
azure-mgmt-policyinsights==1.1.0b2
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the SDK package is a beta version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the latest python sdk version https://pypi.org/project/azure-mgmt-policyinsights/1.1.0b2/

Copy link
Contributor

@zhoxing-ms zhoxing-ms Apr 27, 2022

Choose a reason for hiding this comment

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

@najagasi In fact, what I want to ask is why the latest SDK version is a beta version, is because it contains some unstable features or big changes?
If so, will it affect the existing commands?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this fixes an issue we had ran into wherein the python sdk was making GET calls for nextLink instead of POST calls. Fix was needed in Policy Insights swagger and
py sdk release

in this PR bumping it to 1.1.0b2 fixes the issue in cli

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Apr 21, 2022

@najagasi Could you address these comments by tomorrow? If not, this PR will not be able to catch up with the release of the sprint, and it can only be postponed to the next sprint (05-24)

@najagasi
Copy link
Contributor Author

Working on the feedback

@najagasi
Copy link
Contributor Author

image
reran and committed

@zhoxing-ms zhoxing-ms changed the title [AzurePolicy] Update policy insights version {PolicyInsights} Update SDK package azure-mgmt-policyinsights to 1.1.0b2 Apr 27, 2022
@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Apr 27, 2022

Why did you remove these yaml files?
Screenshot 2022-04-27 113525

In addition, there are still some tests that fail in CI due to the api-version of Microsoft.PolicyInsights for _remediations_operations used by SDK package is upgraded from 2019-07-01 to 2021-10-01, which require you to record in live mode and then push it to this PR:

FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation_complete
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation_management_group
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation_policy_set
FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights_remediation_reevaluate

Namrata Jagasia added 2 commits May 5, 2022 11:00
@najagasi
Copy link
Contributor Author

najagasi commented May 5, 2022

@zhoxing-ms I notive that my tests are passing in live mode in local but when it seems like its failing here.
Any suggestions ?

@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented May 6, 2022

Please note that the api-version from yaml file you uploaded is not updated. You may need to make sure that the 1.1.0b2 version package is installed correctly in your local environment
Screenshot 2022-05-06 142014

Please make sure that the api-version you use when executing the test locally in live mode is 2021-10-01 instead of the old api-version 2019-07-01

@najagasi
Copy link
Contributor Author

@zhoxing-ms thanks for your feeback, i updated the version in my local and reran the tests.
What else might I be missing ?

@zhoxing-ms
Copy link
Contributor

FAILED src/azure-cli/azure/cli/command_modules/policyinsights/tests/latest/test_policyinsights_scenario.py::PolicyInsightsTests::test_policy_insights

@najagasi Please re-record the yaml file for this test test_policy_insights

@najagasi
Copy link
Contributor Author

najagasi commented May 10, 2022

i notice this CannotOverwriteExistingCassetteException, how can I resolve this ?

I generally run this command in my local, once the test is passed I add @record_only on the test and then push it.
azdev test test_policy_insights --live

Is there any other command or flow that i should follow ?

@zhoxing-ms
Copy link
Contributor

@najagasi Did you temporarily remove the @record_only when re-recording the yaml file of test_policy_insights?

@najagasi
Copy link
Contributor Author

Yes i deleted the yaml file, removed the @record only, ran the test with live, add the @record only.

Comment on lines 29 to 39
'--resource "/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/cheggpolicy/providers/microsoft.keyvault/vaults/cheggtmpkv"',
'--resource "cheggkv" --namespace "microsoft.keyvault" --resource-type "vaults" -g "cheggpolicy"',
'--resource "subnet2" --namespace "microsoft.network" --resource-type "subnets" --parent "virtualnetworks/cheggvnet" -g "cheggpolicy"',
'-s "1f3afdf9-d0c9-4c3d-847f-89da613e70a8"',
'-d "34c877ad-507e-4c82-993e-3452a6e0ad3c"',
'-a "4d31128e32d04a0098fd536e"',
'-a "f32eeddfc32345b585f9a70b" -g "cheggpolicy" '
'-g {resourceGroup}',
'--resource "/subscriptions/{subscriptionId}/resourcegroups/{resourceGroup}/providers/microsoft.keyvault/vaults/{keyVault}"',
Copy link
Contributor

@zhoxing-ms zhoxing-ms May 12, 2022

Choose a reason for hiding this comment

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

@najagasi Why do you want to replace the subscription id from 00000000-0000-0000-0000-000000000000 to 3593b919-b078-4cc1-902f-201232a97ac0? Please don't replace it and try again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this fixed it, thank you!

@najagasi
Copy link
Contributor Author

Please do not merge this yet

@najagasi najagasi changed the title {PolicyInsights} Update SDK package azure-mgmt-policyinsights to 1.1.0b2 [do-not-merge]{PolicyInsights} Update SDK package azure-mgmt-policyinsights to 1.1.0b2 May 12, 2022
@najagasi
Copy link
Contributor Author

We are good to merge this

@najagasi najagasi changed the title [do-not-merge]{PolicyInsights} Update SDK package azure-mgmt-policyinsights to 1.1.0b2 {PolicyInsights} Update SDK package azure-mgmt-policyinsights to 1.1.0b2 May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants