Added functionality to create consumption gpu app #8399
Conversation
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
Hi @harryli0108, |
|
Hi @harryli0108, |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
🚫All pull requests will be blocked to merge until Jan 6, 2025 due to CCOA |
microsoft-github-policy-service
bot
requested review from
yonzhan,
wangzelin007,
yanzhudd,
zhoxing-ms,
jsntcy,
ruslany,
sanchitmehta,
ebencarek,
JennyLawrance,
howang-ms,
vinisoto,
chinadragon0515,
vturecek,
torosent,
pagariyaalok,
Juliehzl,
jijohn14 and
Greedygre
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 @harryli0108 Release SuggestionsModule: containerapp
Notes
|
| @@ -698,6 +708,47 @@ def set_up_system_assigned_identity_as_default_if_using_acr(self): | |||
| return | |||
| self.set_argument_registry_identity('system') | |||
|
|
|||
| def set_up_consumption_gpu_wp_payload(self, consumption_gpu_profile_type): | |||
|
|
||
| def validate_consumption_gpu_profile(self): | ||
| if self.get_argument_enable_consumption_gpu() is not None: | ||
| if self.get_argument_enable_consumption_gpu().lower() not in ["consumption-gpu-nc8as-t4", "consumption-gpu-nc4as-t4", "consumption-gpu-nc24-a100", "consumption-gpu-nc12-a100", "consumption-gpu-nv6ads-a10"]: |
There was a problem hiding this comment.
Can you move this to the params and use the built-in validation?
c.argument('consumption_gpu_profile', arg_type=get_enum_type(['consumption-gpu-nc8as-t4', 'consumption-gpu-nc4as-t4', 'consumption-gpu-nc24-a100'...
Example:
|
/azp run |
|
Pull request contains merge conflicts. |
| consumption_gpu_profile_type_lower = consumption_gpu_profile_type.lower() | ||
| if consumption_gpu_profile_type_lower == "consumption-gpu-nc8as-t4": | ||
| payload = { | ||
| "workloadProfileType": "Consumption-GPU-NC8as-T4", | ||
| "name": "consumption-8core-t4" | ||
| } | ||
| elif consumption_gpu_profile_type_lower == "consumption-gpu-nc4as-t4": | ||
| payload = { | ||
| "workloadProfileType": "Consumption-GPU-NC4as-T4", | ||
| "name": "consumption-4core-t4" | ||
| } | ||
| elif consumption_gpu_profile_type_lower == "consumption-gpu-nc24-a100": | ||
| payload = { | ||
| "workloadProfileType": "Consumption-GPU-NC24-A100", | ||
| "name": "consumption-24core-a100" | ||
| } | ||
| elif consumption_gpu_profile_type_lower == "consumption-gpu-nc12-a100": | ||
| payload = { | ||
| "workloadProfileType": "Consumption-GPU-NC12-A100", | ||
| "name": "consumption-12core-a100" | ||
| } |
There was a problem hiding this comment.
Currently when we want to support a new type of workload profile, we don't need to update CLI.
These hard codes are coupled to the workload profile type, which is not conducive to future expansion.
There was a problem hiding this comment.
Do we really need add functionality to create gpu app for az containerapp create?
Currently we use az containerapp create --workload-profile-name to specify the workload profile to run the app on.
This --consumption-gpu-profile has conflict with it.
There was a problem hiding this comment.
@harryli0108
I think it would be better to use az containerapp up for adding functionality to create gpu app.
We can prepare the managed environment workload profile in az containerapp up prepare logic.
Command
az containerapp up : Create or update a container app as well as any associated resources (ACR,
resource group, container apps environment, GitHub Actions, etc.).
| if consumption_gpu_wp_name is None: | ||
| env_client = self.get_environment_client | ||
| wp_payload, consumption_gpu_wp_name = self.update_consumption_gpu_wp(managed_env_info, consumption_gpu_wp) | ||
| env_client().update(cmd=self.cmd, resource_group_name=managed_env_rg, name=managed_env_name, managed_environment_envelope=wp_payload) |
There was a problem hiding this comment.
If we update environment here, the --no-wait will not work for this case, because we have to wait for the update to complete.
There was a problem hiding this comment.
We should warning customer the --no-wait not take effect for this case:
--no-wait will not take effect when using --consumption-gpu-profile and need to create a workload profile.
| consumption_gpu_wp_name = wp["name"] | ||
| break | ||
| if consumption_gpu_wp_name is None: | ||
| env_client = self.get_environment_client |
There was a problem hiding this comment.
| env_client = self.get_environment_client | |
| env_client = self.get_environment_client() |
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az containerapp createGeneral 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.