[containerapp] az containerapp env workload-profile add Simplify wp creation with default profile name
#32713
Conversation
jepetty
requested review from
howang-ms,
yanzhudd and
zhoxing-ms
as code owners
️✔️AzureCLI-FullTest
|
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| containerapp env workload-profile add | cmd containerapp env workload-profile add update parameter workload_profile_name: removed property required=True |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
JennyLawrance,
chinadragon0515,
ebencarek,
jijohn14,
jsntcy,
pagariyaalok,
ruslany,
sanchitmehta,
torosent,
vinisoto,
vturecek,
wangzelin007 and
yonzhan
There was a problem hiding this comment.
Pull request overview
This pull request simplifies the creation of flex workload profiles in Azure Container Apps by making the --workload-profile-name parameter optional when the profile type is "flex". The API automatically assigns a default name for flex profiles, removing the need for users to provide one manually.
Changes:
- Made
workload_profile_nameparameter optional in theadd_workload_profilefunction - Added validation to require
--workload-profile-nameonly for non-flex workload profile types - Updated workload profile setup logic to handle None values for profile names
- Added comprehensive test case to verify flex profiles can be created without names and non-flex profiles still require names
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| custom.py | Made workload_profile_name parameter optional with default value None, updated duplicate name check to handle None |
| containerapp_env_decorator.py | Added validation requiring profile names only for non-flex types, updated profile matching logic to handle None names |
| test_containerapp_workload_profile_commands.py | Added test verifying flex profiles work without names and non-flex profiles fail without names |
Comments suppressed due to low confidence (1)
src/azure-cli/azure/cli/command_modules/containerapp/containerapp_env_decorator.py:311
- The comment says "flex profiles will be named automatically" but this is an implementation detail of the API, not the CLI. Consider clarifying that the API (not the CLI) assigns the default name, e.g., "workload-profile-name is optional for flex profiles as the API will assign a default name automatically".
# validate workload profile arguments - flex profiles will be named automatically
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/azure-cli/azure/cli/command_modules/containerapp/containerapp_env_decorator.py
Outdated
Show resolved
Hide resolved
...cli/command_modules/containerapp/tests/latest/test_containerapp_workload_profile_commands.py
Show resolved
Hide resolved
src/azure-cli/azure/cli/command_modules/containerapp/containerapp_env_decorator.py
Outdated
Show resolved
Hide resolved
...cli/command_modules/containerapp/tests/latest/test_containerapp_workload_profile_commands.py
Show resolved
Hide resolved
src/azure-cli/azure/cli/command_modules/containerapp/containerapp_env_decorator.py
Show resolved
Hide resolved
|
Hi @jepetty Which command this PR for? I reviewed this PR, the change is related to command Please add more test case and description for the PR change. |
|
|
||
| if workload_profile_name: | ||
| # We have already validated that the configuration is valid if adding a workload profile | ||
| if workload_profile_name or workload_profile_type: |
There was a problem hiding this comment.
Does this change want support command only input --workload-profile-type without --workload-profile-name?
There was a problem hiding this comment.
Yes, the name is optional for Flex (but if set it must be Flex, just like Consumption), just trying to simplify the commands. What if we made the name optional for all profiles and set it to the same as the type?
--workload-profile-type D4 creates a profile with name: D4, type: D4.
There was a problem hiding this comment.
@Tratcher I was wondering yesterday why we don't do this for every type. I'll submit a PR to the API if you agree this is the right design
There was a problem hiding this comment.
@Greedygre / @Tratcher, we followed up with @simonjj and have agreed to make this the default behavior from the CLI for all workload-profile types. If the type is provided but not the name, and there's not an existing profile with the same name, we'll create a new WP with name == type. i.e. az containerapp env workload-profile add -n d4-env -g d4-rg --workload-profile-type D4 would create a new WP with name "d4" and type "D4". I will update accordingly
src/azure-cli/azure/cli/command_modules/containerapp/containerapp_env_decorator.py
Outdated
Show resolved
Hide resolved
|
Please fix CI issues |
jepetty
changed the title
az containerapp env workload-profile -n <name> -g <resourceGroup> --workload-profile-type flex Simplify flex wp creationaz containerapp env workload-profile add -n <n> -g <g> --workload-profile-type <type> Simplify wp creation
Sorry for the confusion, @Greedygre, you're right, I definitely missed some words in the description 😅 Updated the description and added some clarity about scope, let me know if you still see missing things that should be added. Thanks for the review! |
jepetty
changed the title
az containerapp env workload-profile add -n <n> -g <g> --workload-profile-type <type> Simplify wp creationaz containerapp env workload-profile add Simplify wp creation with default profile name
5 participants
Related command
Description
This simplifies CLI invocation when creating a new workload-profile for ACA without a name. We will try to provide an intelligent default (i.e. the type of the workload profile)
Testing Guide
This will create two workload profiles:
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.