-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Depricate ingress scale settings #9026
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
Conversation
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| containerapp env premium-ingress add | cmd containerapp env premium-ingress add update parameter max_replicas: added property deprecate_info_expiration=2.78.0 |
||
| containerapp env premium-ingress add | cmd containerapp env premium-ingress add update parameter max_replicas: added property deprecate_info_hide=True |
||
| containerapp env premium-ingress add | cmd containerapp env premium-ingress add update parameter max_replicas: added property deprecate_info_target=max_replicas |
||
| containerapp env premium-ingress add | cmd containerapp env premium-ingress add update parameter max_replicas: removed property required=True |
||
| containerapp env premium-ingress add | cmd containerapp env premium-ingress add update parameter min_replicas: added property deprecate_info_expiration=2.78.0 |
||
| containerapp env premium-ingress add | cmd containerapp env premium-ingress add update parameter min_replicas: added property deprecate_info_hide=True |
||
| containerapp env premium-ingress add | cmd containerapp env premium-ingress add update parameter min_replicas: added property deprecate_info_target=min_replicas |
||
| containerapp env premium-ingress add | cmd containerapp env premium-ingress add update parameter min_replicas: removed property required=True |
||
| containerapp env premium-ingress update | cmd containerapp env premium-ingress update update parameter max_replicas: added property deprecate_info_expiration=2.78.0 |
||
| containerapp env premium-ingress update | cmd containerapp env premium-ingress update update parameter max_replicas: added property deprecate_info_hide=True |
||
| containerapp env premium-ingress update | cmd containerapp env premium-ingress update update parameter max_replicas: added property deprecate_info_target=max_replicas |
||
| containerapp env premium-ingress update | cmd containerapp env premium-ingress update update parameter min_replicas: added property deprecate_info_expiration=2.78.0 |
||
| containerapp env premium-ingress update | cmd containerapp env premium-ingress update update parameter min_replicas: added property deprecate_info_hide=True |
||
| containerapp env premium-ingress update | cmd containerapp env premium-ingress update update parameter min_replicas: added property deprecate_info_target=min_replicas |
|
Hi @Tratcher, |
|
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>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR deprecates the --min-replicas and --max-replicas parameters from the az containerapp env premium-ingress commands due to a design change where ingress will now scale to one large instance per workload profile node instead of using these explicit replica settings.
- Deprecated the
min_replicasandmax_replicasparameters in the command definitions - Removed scale-related logic from the add and update functions for premium ingress
- Updated tests to remove assertions for scale properties and removed scale parameters from test commands
Reviewed Changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
_params.py |
Added deprecation warnings to min_replicas and max_replicas parameters |
custom.py |
Removed scale configuration logic from add/update premium ingress functions |
test_containerapp_commands.py |
Updated tests to remove scale parameters and related assertions |
_help.py |
Updated help examples to remove deprecated scale parameters |
HISTORY.rst |
Added changelog entry documenting the parameter deprecation |
CodeGen Tools Feedback CollectionThank you for using our CodeGen tool. We value your feedback, and we would like to know how we can improve our product. Please take a few minutes to fill our codegen survey |
|
Hi @Tratcher Release SuggestionsModule: containerapp
Notes
|
| env_patch = {} | ||
| ingress_config = {} | ||
| safe_set(env_patch, "properties", "ingressConfiguration", value=ingress_config) | ||
| scale = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls keep the code until parameter is removed. Currently they are in deprecated mode and hidden to customers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The control plane change has already fully deployed, the properties are ignored and there's no reason to send them.
Co-authored-by: Copilot <[email protected]> Co-authored-by: Zunli Hu <[email protected]>
| c.argument('workload_profile_name', options_list=['--workload-profile-name', '-w'], help="The workload profile to run ingress replicas on. This profile must not be shared with any container app or job.") | ||
| c.argument('min_replicas', options_list=['--min-replicas'], type=int, help="Minimum number of replicas to run. Default 2, minimum 2.") | ||
| c.argument('max_replicas', options_list=['--max-replicas'], type=int, help="Maximum number of replicas to run. Default 10. The upper limit is the maximum cores available in the workload profile.") | ||
| c.argument('min_replicas', options_list=['--min-replicas'], type=int, deprecate_info=c.deprecate(hide=True, expiration='1.3.0b1'), help="The workload profile minimum instances is used instead.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter: containerapp env premium-ingress add,
max_replicas- Deprecated parameter is expired and should be removed.
??
|
Please fix CI issues |
|
@yonzhan The error seems incorrect, do you know who can help troubleshoot the deprecate logic? |
|
Hi @Tratcher This is cause by The version value for About the integration tests failed. You can refer the CLI test doc:
|
|
I've reverted the test file changes, they're unrelated to this PR. |
|
Hi @Juliehzl |
Due to a design change these two parameters are no longer needed. Ingress will instead scale to one large instance per workload profile node. This redoes #9023 with deprecation instead of removal.
Contributes to https://msazure.visualstudio.com/Antares/_workitems/edit/33117299
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az containerapp env premium-ingress add/update
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)