-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[Container app] az containerapp env java-component: Support new configuration parameters on java component #8320
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 java-component admin-for-spring create | cmd containerapp env java-component admin-for-spring create added parameter set_configurations |
||
| containerapp env java-component admin-for-spring create | cmd containerapp env java-component admin-for-spring create update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component admin-for-spring create | cmd containerapp env java-component admin-for-spring create update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component admin-for-spring create | cmd containerapp env java-component admin-for-spring create update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component admin-for-spring update | cmd containerapp env java-component admin-for-spring update added parameter remove_all_configurations |
||
| containerapp env java-component admin-for-spring update | cmd containerapp env java-component admin-for-spring update added parameter remove_configurations |
||
| containerapp env java-component admin-for-spring update | cmd containerapp env java-component admin-for-spring update added parameter replace_configurations |
||
| containerapp env java-component admin-for-spring update | cmd containerapp env java-component admin-for-spring update added parameter set_configurations |
||
| containerapp env java-component admin-for-spring update | cmd containerapp env java-component admin-for-spring update update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component admin-for-spring update | cmd containerapp env java-component admin-for-spring update update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component admin-for-spring update | cmd containerapp env java-component admin-for-spring update update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component config-server-for-spring create | cmd containerapp env java-component config-server-for-spring create added parameter set_configurations |
||
| containerapp env java-component config-server-for-spring create | cmd containerapp env java-component config-server-for-spring create update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component config-server-for-spring create | cmd containerapp env java-component config-server-for-spring create update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component config-server-for-spring create | cmd containerapp env java-component config-server-for-spring create update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component config-server-for-spring update | cmd containerapp env java-component config-server-for-spring update added parameter remove_all_configurations |
||
| containerapp env java-component config-server-for-spring update | cmd containerapp env java-component config-server-for-spring update added parameter remove_configurations |
||
| containerapp env java-component config-server-for-spring update | cmd containerapp env java-component config-server-for-spring update added parameter replace_configurations |
||
| containerapp env java-component config-server-for-spring update | cmd containerapp env java-component config-server-for-spring update added parameter set_configurations |
||
| containerapp env java-component config-server-for-spring update | cmd containerapp env java-component config-server-for-spring update update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component config-server-for-spring update | cmd containerapp env java-component config-server-for-spring update update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component config-server-for-spring update | cmd containerapp env java-component config-server-for-spring update update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component eureka-server-for-spring create | cmd containerapp env java-component eureka-server-for-spring create added parameter set_configurations |
||
| containerapp env java-component eureka-server-for-spring create | cmd containerapp env java-component eureka-server-for-spring create update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component eureka-server-for-spring create | cmd containerapp env java-component eureka-server-for-spring create update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component eureka-server-for-spring create | cmd containerapp env java-component eureka-server-for-spring create update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component eureka-server-for-spring update | cmd containerapp env java-component eureka-server-for-spring update added parameter remove_all_configurations |
||
| containerapp env java-component eureka-server-for-spring update | cmd containerapp env java-component eureka-server-for-spring update added parameter remove_configurations |
||
| containerapp env java-component eureka-server-for-spring update | cmd containerapp env java-component eureka-server-for-spring update added parameter replace_configurations |
||
| containerapp env java-component eureka-server-for-spring update | cmd containerapp env java-component eureka-server-for-spring update added parameter set_configurations |
||
| containerapp env java-component eureka-server-for-spring update | cmd containerapp env java-component eureka-server-for-spring update update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component eureka-server-for-spring update | cmd containerapp env java-component eureka-server-for-spring update update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component eureka-server-for-spring update | cmd containerapp env java-component eureka-server-for-spring update update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component gateway-for-spring create | cmd containerapp env java-component gateway-for-spring create added parameter service_bindings |
||
| containerapp env java-component gateway-for-spring create | cmd containerapp env java-component gateway-for-spring create added parameter set_configurations |
||
| containerapp env java-component gateway-for-spring create | cmd containerapp env java-component gateway-for-spring create added parameter unbind_service_bindings |
||
| containerapp env java-component gateway-for-spring create | cmd containerapp env java-component gateway-for-spring create update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component gateway-for-spring create | cmd containerapp env java-component gateway-for-spring create update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component gateway-for-spring create | cmd containerapp env java-component gateway-for-spring create update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update added parameter remove_all_configurations |
||
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update added parameter remove_configurations |
||
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update added parameter replace_configurations |
||
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update added parameter service_bindings |
||
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update added parameter set_configurations |
||
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update added parameter unbind_service_bindings |
||
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component gateway-for-spring update | cmd containerapp env java-component gateway-for-spring update update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component nacos create | cmd containerapp env java-component nacos create added parameter set_configurations |
||
| containerapp env java-component nacos create | cmd containerapp env java-component nacos create update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component nacos create | cmd containerapp env java-component nacos create update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component nacos create | cmd containerapp env java-component nacos create update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component nacos update | cmd containerapp env java-component nacos update added parameter remove_all_configurations |
||
| containerapp env java-component nacos update | cmd containerapp env java-component nacos update added parameter remove_configurations |
||
| containerapp env java-component nacos update | cmd containerapp env java-component nacos update added parameter replace_configurations |
||
| containerapp env java-component nacos update | cmd containerapp env java-component nacos update added parameter set_configurations |
||
| containerapp env java-component nacos update | cmd containerapp env java-component nacos update update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component nacos update | cmd containerapp env java-component nacos update update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component nacos update | cmd containerapp env java-component nacos update update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component spring-cloud-config create | cmd containerapp env java-component spring-cloud-config create added parameter set_configurations |
||
| containerapp env java-component spring-cloud-config create | cmd containerapp env java-component spring-cloud-config create update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component spring-cloud-config create | cmd containerapp env java-component spring-cloud-config create update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component spring-cloud-config create | cmd containerapp env java-component spring-cloud-config create update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component spring-cloud-config update | cmd containerapp env java-component spring-cloud-config update added parameter remove_all_configurations |
||
| containerapp env java-component spring-cloud-config update | cmd containerapp env java-component spring-cloud-config update added parameter remove_configurations |
||
| containerapp env java-component spring-cloud-config update | cmd containerapp env java-component spring-cloud-config update added parameter replace_configurations |
||
| containerapp env java-component spring-cloud-config update | cmd containerapp env java-component spring-cloud-config update added parameter set_configurations |
||
| containerapp env java-component spring-cloud-config update | cmd containerapp env java-component spring-cloud-config update update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component spring-cloud-config update | cmd containerapp env java-component spring-cloud-config update update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component spring-cloud-config update | cmd containerapp env java-component spring-cloud-config update update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component spring-cloud-eureka create | cmd containerapp env java-component spring-cloud-eureka create added parameter set_configurations |
||
| containerapp env java-component spring-cloud-eureka create | cmd containerapp env java-component spring-cloud-eureka create update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component spring-cloud-eureka create | cmd containerapp env java-component spring-cloud-eureka create update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component spring-cloud-eureka create | cmd containerapp env java-component spring-cloud-eureka create update parameter configuration: added property deprecate_info_target=--configuration |
||
| containerapp env java-component spring-cloud-eureka update | cmd containerapp env java-component spring-cloud-eureka update added parameter remove_all_configurations |
||
| containerapp env java-component spring-cloud-eureka update | cmd containerapp env java-component spring-cloud-eureka update added parameter remove_configurations |
||
| containerapp env java-component spring-cloud-eureka update | cmd containerapp env java-component spring-cloud-eureka update added parameter replace_configurations |
||
| containerapp env java-component spring-cloud-eureka update | cmd containerapp env java-component spring-cloud-eureka update added parameter set_configurations |
||
| containerapp env java-component spring-cloud-eureka update | cmd containerapp env java-component spring-cloud-eureka update update parameter configuration: added property deprecate_info_hide=True |
||
| containerapp env java-component spring-cloud-eureka update | cmd containerapp env java-component spring-cloud-eureka update update parameter configuration: added property `deprecate_info_redirect=--[set |
replace | |
| containerapp env java-component spring-cloud-eureka update | cmd containerapp env java-component spring-cloud-eureka update update parameter configuration: added property deprecate_info_target=--configuration |
|
Hi @Descatles, |
|
Hi @Descatles, |
|
containerapp |
| c.argument('unbind_service_bindings', nargs='*', options_list=['--unbind'], help="Space separated list of services, bindings or Java components to be removed from this Java Component. e.g. BIND_NAME1...") | ||
| c.argument('configuration', nargs="*", help="Java component configuration. Configuration must be in format \"<propertyName>=<value>\" \"<propertyName>=<value>\"...") | ||
| c.argument('configuration', nargs="*", help="Java component configuration. Configuration must be in format \"<propertyName>=<value>\" \"<propertyName>=<value>\"...", deprecate_info=c.deprecate(target="--configuration")) | ||
| c.argument('set_configurations', nargs="*", options_list=['--set-configurations', '--set-configs'], help="Add or update Java component configuration(s). Configurations must be in format \"<propertyName>=<value>\" \"<propertyName>=<value>\"...") |
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.
I noticed that the env_vars help text for set is --set-env-vars : Add or update environment variable(s) in container. Existing environment variables are not modified.
Can we add one more similar line: Other existing configurations are not modified ?
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.
Sure, updated this message
| c.argument('service_bindings', nargs='*', options_list=['--bind'], help="Space separated list of services, bindings or other Java components to be connected to this Java Component. e.g. SVC_NAME1[:BIND_NAME1] SVC_NAME2[:BIND_NAME2]...") | ||
| c.argument('unbind_service_bindings', nargs='*', options_list=['--unbind'], help="Space separated list of services, bindings or Java components to be removed from this Java Component. e.g. BIND_NAME1...") | ||
| c.argument('configuration', nargs="*", help="Java component configuration. Configuration must be in format \"<propertyName>=<value>\" \"<propertyName>=<value>\"...") | ||
| c.argument('configuration', nargs="*", help="Java component configuration. Configuration must be in format \"<propertyName>=<value>\" \"<propertyName>=<value>\"...", deprecate_info=c.deprecate(target="--configuration")) |
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.
Suggest to add redirect="--[set|replace|remove|remove-all]-configurations", and probably hide=True
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.
sure. added
|
|
||
| def validate_configurations(self): | ||
| if self.set_configuration_with_legacy_way() and self.set_configuration_with_new_way(): | ||
| raise ValidationError("--configuration could not be specify alongside any of the following options: --set-configurations, --replace-configurations, --remove-configurations, or --remove-all-configurations. Use either --configuration alone or other mentioned parameters as needed.") |
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.
Maybe better to say use --[set|replace|remove|remove-all]-configurations. No need to hint for deprecated usage.
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.
Sure, I have updated the message to The --configuration option cannot be used together with --[set|replace|remove|remove-all]-configurations. Please use --[set|replace|remove|remove-all]-configurations for better flexibility and clarity.
| if existing_configuration["propertyName"].lower() == new_configuration["propertyName"].lower(): | ||
| is_existing = True | ||
|
|
||
| if "value" in new_configuration: |
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.
Given that we will always set the new configuration. Shall we check the presence of ["value"] to the outer for loop as a validation? I presume if this results in add, we still need to ensure that ["value"] is present.
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.
Yes, it's better to put the check out the loop. thanks!
| if existing_configuration["propertyName"].lower() == remove_configuration.lower(): | ||
| is_existing = True | ||
| existing_configurations.pop(i) | ||
| break |
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.
Are there cases that the existing configuration has duplicate property names?
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.
Good point, I updated the parse configurations function to ensure there will be no duplicate property names passed and also delete the break to avoid duplicate from other ways like portal
| def set_up_replace_configurations(self): | ||
| if self.get_argument_replace_configurations() is not None: | ||
| configuration_list = self.parse_configurations(self.get_argument_replace_configurations()) | ||
| self.java_component_def["properties"]["configurations"] = [] |
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.
just set it to configuration_list and skip the next method call?
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.
Cool, that's better, updated
| self.set_up_unbind_service_bindings() | ||
| self.set_up_gateway_route() | ||
| self.set_up_replicas() | ||
| self.validate_configurations() |
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.
We only checked that old style configuration settings cannot be specified together with the new style ones. What if the customer specify both set and replace (and possibly other combinations)? Can we check and align the ACA --xxx-env-vars behavior?
My gut feeling is that we should only allow the following combination and reject all other cases:
- set + remove
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.
In the ACA code, the four parameters can be used together, with operations executed in the following sequence: set, replace, remove, and remove-all. For example, replace will override all configurations specified by set, and remove-all could also remove all configurations specified by replace
| -n MyJavaComponentName \\ | ||
| --environment MyEnvironment \\ | ||
| --configuration | ||
| --remove-all-configurations |
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.
we'd better add some examples about how to use --remove-configurations
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.
sure, examples added
| -n MyJavaComponentName \\ | ||
| --environment MyEnvironment \\ | ||
| --configuration PropertyName1=Value1 PropertyName2=Value2 | ||
| --set-configurations PropertyName1=Value1 PropertyName2=Value2 |
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.
Should we add some examples for this parameter --replace-configurations to describe how to use it?
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.
sure, examples added
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| - name: Delete all configurations of the Config Server for Spring. | ||
| text: | | ||
| az containerapp env java-component config-server-for-spring update -g MyResourceGroup \\ | ||
| -n MyJavaComponentName \\ | ||
| --environment MyEnvironment \\ | ||
| --remove-all-configurations |
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.
Why this is dup with line 1355-1358?
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.
Dup removed
|
Hi @Descatles Release SuggestionsModule: containerapp
Notes
|
| - name: Delete all configurations of the Eureka Server for Spring. | ||
| text: | | ||
| az containerapp env java-component eureka-server-for-spring update -g MyResourceGroup \\ | ||
| -n MyJavaComponentName \\ | ||
| --environment MyEnvironment \\ | ||
| --remove-all-configurations |
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.
dup with line 1498-1501?
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.
Dup removed
| - name: Delete all configurations of the Admin for Spring. | ||
| text: | | ||
| az containerapp env java-component admin-for-spring update -g MyResourceGroup \\ | ||
| -n MyJavaComponentName \\ | ||
| --environment MyEnvironment \\ | ||
| --remove-all-configurations |
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.
dup?
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.
Dup removed
| - name: Delete all configurations of the nacos. | ||
| text: | | ||
| az containerapp env java-component nacos update -g MyResourceGroup \\ | ||
| -n MyJavaComponentName \\ | ||
| --environment MyEnvironment \\ | ||
| --remove-all-configurations |
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.
dup?
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.
Dup removed
| c.argument('remove_configurations', nargs="*", options_list=['--remove-configurations', '--remove-configs'], help="Remove Java component configuration(s). Specify configuration names separated by space, in format `<propertyName>` `<propertyName>`...") | ||
| c.argument('remove_all_configurations', arg_type=get_three_state_flag(), options_list=['--remove-all-configurations', '--remove-all-configs'], help="Remove all Java component configuration(s).") |
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.
remove_configurations
remove_all_configurations
Because these two parameters are similar, can they be merged into one parameter? For example, when --remove-configurations does not specify a specific value, the all the configurations are deleted
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.
Since az containerapp includes parameters like [--remove-all-env-vars] and [--remove-env-vars], I think we should align with that.
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.
OK, got it~
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
…guration parameters on java component (Azure#8320) * Support new configuration parameters on java component * Add test file * Add recording * remove cred * test fix * test fix * Fix lint * convert recording * Fix * Update recording * udpate recording * Update * resolve comments * format update * Update * Update * Add --bind --unbind in gateway create/update * Fix error help message * Update according to comments * Update according to comments * fix typo * Update * Add examples * Remove duplicated examples --------- Co-authored-by: Wenhao Zhang <[email protected]>
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az containerapp env java-componentSupport more flexible configuration updates with new parameters
--set-configurations,--replace-configurations,--remove-configurationsand--remove-all-configurationsaz containerapp env java-component gateway-for-springSupport
--bind,--unbindwith minor updateGeneral Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.