Skip to content

{aro} az aro module migration to Microsoft Graph#22549

Merged
jiasli merged 2 commits intoAzure:devfrom
nwnt:aro-msgraph
May 31, 2022
Merged

{aro} az aro module migration to Microsoft Graph#22549
jiasli merged 2 commits intoAzure:devfrom
nwnt:aro-msgraph

Conversation

@nwnt
Copy link
Copy Markdown
Contributor

@nwnt nwnt commented May 20, 2022 •
edited
Loading

Related command
az aro

Description
Migrating from Azure SDK's AAD Graph to Microsoft Graph
#22174

Testing Guide
All the commands were executed successfully.

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

@ghost ghost requested review from jiasli and yonzhan May 20, 2022 18:31
@ghost ghost added the Auto-Assign Auto assign by bot label May 20, 2022
@ghost ghost requested a review from wangzelin007 May 20, 2022 18:31
@ghost ghost assigned jiasli May 20, 2022
@ghost ghost added this to the Jun 2022 (2022-07-05) milestone May 20, 2022
@ghost ghost added the Graph (doesn't work with label-triggered comments; use Graph.Microsoft instead) az ad label May 20, 2022
@ghost ghost requested review from calvinhzy and evelyn-ys May 20, 2022 18:31
@ghost ghost added the AAD label May 20, 2022
@nwnt
Copy link
Copy Markdown
Contributor Author

nwnt commented May 20, 2022 •
edited
Loading

@jiasli I was unable to re-record new set of live tests - randomly got 404 id not found error after the app was created. As you can see, 8580f82d-d1c6-43bb-af15-57a710bdbc5b does exist.

The behavior was so random as well. The same step is used in different test cases, and sometimes one test would execute successfully, the other will not. This is very very weird.

self = <azure.cli.command_modules.role.msgrpah._graph_client.GraphClient object at 0x7f65633d9e70>
method = 'POST'
url = 'https://graph.microsoft.com/v1.0/applications/8580f82d-d1c6-43bb-af15-57a710bdbc5b/addPassword'
param = None, body = {}

    def _send(self, method, url, param=None, body=None):
        url = self.base_url + url



        if body:
            body = json.dumps(body)

        list_result = []
        is_list_result = False

        while True:
            try:
                r = send_raw_request(self.cli_ctx, method, url, resource=self.resource, uri_parameters=param, body=body)
            except CLIError as ex:
>               raise GraphError(ex.response.json()['error']['message'], ex.response) from ex
E               azure.cli.command_modules.role.msgrpah._graph_client.GraphError: Resource '8580f82d-d1c6-43bb-af15-57a710bdbc5b' does not exist or one of its queried reference-property objects are not present.

../../../role/msgrpah/_graph_client.py:40: GraphError

Untitled

@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented May 20, 2022

Graph

jiasli
jiasli reviewed May 23, 2022
View reviewed changes
jiasli
jiasli reviewed May 23, 2022
View reviewed changes
jiasli
jiasli reviewed May 23, 2022
View reviewed changes
jiasli
jiasli reviewed May 23, 2022
View reviewed changes
@jiasli
Copy link
Copy Markdown
Member

jiasli commented May 23, 2022 •
edited
Loading

@Nontw, I have looped you in the internal email for this 404 issue.

The best way to solve it is to time.sleep for a while, like I am doing in role module:

azure-cli/src/azure-cli/azure/cli/command_modules/role/tests/latest/test_role.py

Line 397 in cac3f06

time.sleep(30)

You don't need to worry about its efficiency since it is mocked out during playback.

azure-cli/src/azure-cli-testsdk/azure/cli/testsdk/scenario_tests/patches.py

Lines 9 to 13 in 30216d0

def patch_time_sleep_api(unit_test):
def _time_sleep_skip(*_):
return
mock_in_unit_test(unit_test, 'time.sleep', _time_sleep_skip)

@nwnt
Copy link
Copy Markdown
Contributor Author

nwnt commented May 25, 2022

Add test recordings.

@jiasli jiasli changed the base branch from beta to dev May 25, 2022 05:25
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented May 25, 2022

Please resolve the conflicting files.

@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented May 25, 2022 •
edited
Loading

Please resolve the conflicting files

@jiasli jiasli merged commit 8e7c613 into Azure:dev May 31, 2022
@jiasli jiasli changed the title {Graph} az aro module migration to Microsoft Graph {aro} az aro module migration to Microsoft Graph Jun 3, 2022
@jiasli jiasli mentioned this pull request Jun 3, 2022
Closed
@jiasli jiasli mentioned this pull request Mar 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jiasli jiasli jiasli approved these changes

@evelyn-ys evelyn-ys evelyn-ys approved these changes

@yonzhan yonzhan Awaiting requested review from yonzhan

@wangzelin007 wangzelin007 Awaiting requested review from wangzelin007

@calvinhzy calvinhzy Awaiting requested review from calvinhzy

@zhoxing-ms zhoxing-ms Awaiting requested review from zhoxing-ms

@jsntcy jsntcy Awaiting requested review from jsntcy jsntcy is a code owner

@DaeunYim DaeunYim Awaiting requested review from DaeunYim

@digimaun digimaun Awaiting requested review from digimaun

@andyliuliming andyliuliming Awaiting requested review from andyliuliming

@kairu-ms kairu-ms Awaiting requested review from kairu-ms

@necusjz necusjz Awaiting requested review from necusjz necusjz is a code owner

@panchagnula panchagnula Awaiting requested review from panchagnula

@dbradish-microsoft dbradish-microsoft Awaiting requested review from dbradish-microsoft

Assignees

@jiasli jiasli

Labels

AAD Auto-Assign Auto assign by bot Graph (doesn't work with label-triggered comments; use Graph.Microsoft instead) az ad

Projects

None yet

Milestone

Jun 2022 (2022-07-05)

Development

Successfully merging this pull request may close these issues.

4 participants

@nwnt @yonzhan @jiasli @evelyn-ys