Skip to content

{IoT} Refactor KeyReplacer folder structure #28462

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.

Sign up for GitHub

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

Jump to bottom
Merged
zhoxing-ms merged 3 commits into from
Feb 28, 2024
Merged

{IoT} Refactor KeyReplacer folder structure #28462

zhoxing-ms merged 3 commits into from
Feb 28, 2024

Conversation

@zhoxing-ms
Copy link
Contributor

@zhoxing-ms zhoxing-ms commented Feb 27, 2024

Related command

Description

Issue background: https://github.com/Azure/azure-cli/pull/28427/files/6a7ac3ae7a96fe122e7a1c2bbcc6a1b78eed3ef4#r1503570453
Similar as: #19285 #17817

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.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Feb 27, 2024 •
edited
Loading

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.11
️✔️3.9
️✔️ams
️✔️latest
️✔️3.11
️✔️3.9
️✔️apim
️✔️latest
️✔️3.11
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.11
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️aro
️✔️latest
️✔️3.11
️✔️3.9
️✔️backup
️✔️latest
️✔️3.11
️✔️3.9
️✔️batch
️✔️latest
️✔️3.11
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.11
️✔️3.9
️✔️billing
️✔️latest
️✔️3.11
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.11
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.11
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.11
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️config
️✔️latest
️✔️3.11
️✔️3.9
️✔️configure
️✔️latest
️✔️3.11
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.11
️✔️3.9
️✔️container
️✔️latest
️✔️3.11
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.11
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.11
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️dla
️✔️latest
️✔️3.11
️✔️3.9
️✔️dls
️✔️latest
️✔️3.11
️✔️3.9
️✔️dms
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.11
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.11
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.11
️✔️3.9
️✔️find
️✔️latest
️✔️3.11
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.11
️✔️3.9
️✔️identity
️✔️latest
️✔️3.11
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.11
️✔️3.9
️✔️lab
️✔️latest
️✔️3.11
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.11
️✔️3.9
️✔️maps
️✔️latest
️✔️3.11
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.11
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.11
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.11
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.11
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.11
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.11
️✔️3.9
️✔️profile
️✔️latest
️✔️3.11
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.11
️✔️3.9
️✔️redis
️✔️latest
️✔️3.11
️✔️3.9
️✔️relay
️✔️latest
️✔️3.11
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️role
️✔️latest
️✔️3.11
️✔️3.9
️✔️search
️✔️latest
️✔️3.11
️✔️3.9
️✔️security
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.11
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.11
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.11
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.11
️✔️3.9
️✔️sql
️✔️latest
️✔️3.11
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.11
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.11
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9
️✔️util
️✔️latest
️✔️3.11
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.9

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Feb 27, 2024

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

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Feb 27, 2024 •
edited
Loading

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Feb 27, 2024

Core

@microsoft-github-policy-service microsoft-github-policy-service bot added the Auto-Assign Auto assign by bot label Feb 27, 2024
@zhoxing-ms zhoxing-ms requested a review from c-ryan-k February 27, 2024 03:15
yonzhan
yonzhan previously approved these changes Feb 27, 2024
View reviewed changes
@zhoxing-ms
Copy link
Contributor Author

zhoxing-ms commented Feb 27, 2024

@c-ryan-k @digimaun Please note that this PR is to address the CI issue caused by the lack of __init__.py (comment link). If you have time to review it, please help review it. If not, in order to avoid blocking the build process of CLI, we will merge it first as it does not affect business logic of IoT module

jiasli
jiasli previously approved these changes Feb 27, 2024
View reviewed changes
@jiasli jiasli mentioned this pull request Feb 27, 2024
Open
yanzhudd
yanzhudd previously approved these changes Feb 27, 2024
View reviewed changes
@jiasli
Copy link
Member

jiasli commented Feb 27, 2024

CI still fails with the same error:

https://dev.azure.com/azclitools/public/_build/results?buildId=134051&view=logs&j=a92c0e78-c951-523a-85be-a933049bed50&t=6e78b194-5635-5321-0acd-26471375c5ca

/opt/az/lib/python3.11/site-packages/azure/cli/command_modules/iot/tests/latest/test_iot_dps_commands.py:13: in <module>
    from azure.cli.command_modules.iot.tests.utils.recording_processors import KeyReplacer
E   ModuleNotFoundError: No module named 'azure.cli.command_modules.iot.tests.utils'

From the CI script, we can see only the azure.cli.command_modules.$name.tests folder is included as packages:

azure-cli/scripts/ci/build.sh

Lines 130 to 138 in a5198b5

for name in `ls src/azure-cli/azure/cli/command_modules`; do
test_folder=src/azure-cli/azure/cli/command_modules/$name/tests
if [ -d $test_folder ]; then
echo " 'azure.cli.command_modules.$name.tests'," >>$testsrc_dir/setup.py
if [ -d $test_folder/$target_profile ]; then
echo " 'azure.cli.command_modules.$name.tests.$target_profile'," >>$testsrc_dir/setup.py
fi
fi
done

as this setup.py doesn't use find_packages, azure.cli.command_modules.$name.tests.utils will not be included.

I suggest putting src/azure-cli/azure/cli/command_modules/iot/tests/utils/recording_processors.py directly under azure.cli.command_modules.$name.tests as util.py, such as

azure-cli/src/azure-cli/azure/cli/command_modules/role/tests/util.py

Lines 1 to 10 in 261c6d5

# --------------------------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------
from knack.util import CLIError
from azure.cli.testsdk.scenario_tests.recording_processors import RecordingProcessor
ROLE_COMMAND_MAX_RETRY = 20
ROLE_COMMAND_SLEEP_DURATION = 10

@zhoxing-ms zhoxing-ms dismissed stale reviews from yanzhudd, jiasli, and yonzhan via 2b087ca February 27, 2024 08:26
@zhoxing-ms zhoxing-ms force-pushed the fix_iot_test_init branch 2 times, most recently from cc4f156 to 02f1544 Compare February 27, 2024 08:33
@zhoxing-ms zhoxing-ms changed the title {Core} Add __init__.py to iot/tests/utils {IoT} Refactor KeyReplacer folder structure Feb 27, 2024
@zhoxing-ms zhoxing-ms merged commit 9c62a21 into Azure:dev Feb 28, 2024
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

@c-ryan-k c-ryan-k c-ryan-k approved these changes

@digimaun digimaun digimaun approved these changes

@yonzhan yonzhan yonzhan left review comments

@yanzhudd yanzhudd yanzhudd left review comments

Assignees

@jiasli jiasli

@zhoxing-ms zhoxing-ms

Labels

Auto-Assign Auto assign by bot Core CLI core infrastructure IoT/CLI IoT

Projects

None yet

Milestone

February 2024 (2024-03-05)

Development

Successfully merging this pull request may close these issues.

6 participants

@zhoxing-ms @yonzhan @jiasli @c-ryan-k @digimaun @yanzhudd