Skip to content

[Container App]: Remove quota check for 'az containerapps env create'#5548

Merged
zhoxing-ms merged 9 commits into
Azure:mainfrom
Zijian-Ju:fixed-containerapps-quota-check
Nov 22, 2022
Merged

[Container App]: Remove quota check for 'az containerapps env create'#5548
zhoxing-ms merged 9 commits into
Azure:mainfrom
Zijian-Ju:fixed-containerapps-quota-check

Conversation

@Zijian-Ju
Copy link
Copy Markdown
Contributor

@Zijian-Ju Zijian-Ju commented Nov 15, 2022


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

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.json automatically.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

@ghost ghost added the Auto-Assign Auto assign by bot label Nov 15, 2022
@ghost ghost requested review from kairu-ms and necusjz November 15, 2022 05:37
@ghost ghost assigned kairu-ms Nov 15, 2022
@ghost ghost added this to the Nov 2022 (2022-12-06) milestone Nov 15, 2022
@ghost ghost requested a review from yonzhan November 15, 2022 05:37
@Zijian-Ju Zijian-Ju changed the title Remove quota check while creating ManagedEnvironments [ContainerApps]: Remove quota check while creating ManagedEnvironments Nov 15, 2022
@Zijian-Ju Zijian-Ju changed the title [ContainerApps]: Remove quota check while creating ManagedEnvironments [Container App]: Remove quota check while creating ManagedEnvironments Nov 15, 2022
@Zijian-Ju Zijian-Ju changed the title [Container App]: Remove quota check while creating ManagedEnvironments [Container App]: Remove quota check for 'az containerapps env create' Nov 15, 2022
Comment thread src/containerapp/azext_containerapp/_utils.py Outdated
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Nov 15, 2022

Container App

@Zijian-Ju Zijian-Ju requested review from Juliehzl and removed request for StrawnSC, kairu-ms, lil131, necusjz, runefa and yonzhan November 15, 2022 07:52
@runefa
Copy link
Copy Markdown
Contributor

runefa commented Nov 15, 2022

This will break az containerapp up logic. Could you instead remove the validate_environment_location call from az containerapp create and leave validate_environment_location as is?

Copy link
Copy Markdown
Contributor

@runefa runefa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave validate_environment_location as-is and simply remove the call of validate_managed_environment from create_managed_environment. containerapp up auto creates environments and therefore needs to ensure the environment is created so it must be sure there is available quota on region using validate_environment_logic. Removing this would result in a worse customer experience.

@yonzhan yonzhan requested a review from zhoxing-ms November 16, 2022 00:24
@yonzhan yonzhan assigned zhoxing-ms and unassigned kairu-ms Nov 16, 2022
@Zijian-Ju
Copy link
Copy Markdown
Contributor Author

@Zijian-Ju Zijian-Ju closed this Nov 16, 2022
@Zijian-Ju Zijian-Ju reopened this Nov 16, 2022
@zhoxing-ms
Copy link
Copy Markdown
Contributor

zhoxing-ms commented Nov 16, 2022

@Zijian-Ju Do you want to release a new extension version for this PR change? If so, please write the description of PR change into the HISTORY.rst and also upgrade the version defined in setup.py

@Zijian-Ju
Copy link
Copy Markdown
Contributor Author

Currently we do allow our customers to create more than 5 managed environments in a region after they submitted a quota request. Otherwise we restrict some of the customers to create less than 5 environments according to their subscription offer type.

In this case, will it still block customers to create environment using 'az container up' when they are allowed to create more?

@Zijian-Ju Zijian-Ju closed this Nov 16, 2022
@Zijian-Ju Zijian-Ju reopened this Nov 16, 2022
Comment thread src/containerapp/azext_containerapp/_utils.py Outdated
Comment thread src/containerapp/azext_containerapp/_up_utils.py Outdated
Comment thread src/containerapp/azext_containerapp/_utils.py Outdated
Comment thread src/containerapp/azext_containerapp/_utils.py Outdated
return location
else:
raise ValidationError("You cannot create any more environments. Environments are limited to {} per location in a subscription. Please specify an existing environment using --environment.".format(MAX_ENV_PER_LOCATION))
return res_locations[0]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add null check

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

res_location will not be empty after the change as we don't filter by remaining quota anymore

@Zijian-Ju
Copy link
Copy Markdown
Contributor Author

Hi @zhoxing-ms, could you please help merge this PR and make a new release? The customer is urgently waiting the fix. Thank you

@zhoxing-ms zhoxing-ms merged commit 3ada38f into Azure:main Nov 22, 2022
@azclibot
Copy link
Copy Markdown
Collaborator

[Release] Update index.json for extension [ containerapp ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=16319&view=results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto-Assign Auto assign by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants