Skip to content

Conversation

@leonard520
Copy link
Contributor


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.
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.

Xiaoyun Ding added 2 commits June 14, 2023 13:47
@azure-client-tools-bot-prd
Copy link

Hi @leonard520,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@ghost ghost requested review from yanzhudd and zhoxing-ms June 14, 2023 05:58
@ghost ghost assigned zhoxing-ms Jun 14, 2023
@yonzhan
Copy link
Collaborator

yonzhan commented Jun 14, 2023

Thank you for your contribution! We will review the pull request and get back to you soon.

@ghost ghost added the Auto-Assign Auto assign by bot label Jun 14, 2023
@ghost ghost requested review from FumingZhang and yonzhan June 14, 2023 05:58
@ghost ghost added the AKS label Jun 14, 2023
@yanzhudd
Copy link
Contributor

Please fix the CI

Comment on lines 5 to 7
* Add new command -- `az spring application-configuration-service create --generation` to support create Application Configuration Service with different generation
* Add new command -- `az spring application-configuration-service update --generation` to support update Application Configuration Service to different generation
* Add new command -- `az spring application-configuration-service git repo add --ca-cert-name` to support bind certificate to Application Configuration Service Gen2
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Add new command -- `az spring application-configuration-service create --generation` to support create Application Configuration Service with different generation
* Add new command -- `az spring application-configuration-service update --generation` to support update Application Configuration Service to different generation
* Add new command -- `az spring application-configuration-service git repo add --ca-cert-name` to support bind certificate to Application Configuration Service Gen2
* Add new command -- `az spring application-configuration-service create --generation` to support creating Application Configuration Service with different generation
* Add new command -- `az spring application-configuration-service update --generation` to support updating Application Configuration Service to different generation
* Add new command -- `az spring application-configuration-service git repo add --ca-cert-name` to support binding certificate to Application Configuration Service Gen2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Comment on lines 161 to 167
c.argument('application_configuration_service_generation',
arg_group="Application Configuration Service",
type=ConfigurationServiceGeneration,
options_list=['--application-configuration-service-generation', '--acs-gen'],
validator=validate_acs_create,
help='(Enterprise Tier Only) Application Configuration Service Generation to enable. Allowed values are: ' + ', '.join(list(ConfigurationServiceGeneration)))
c.argument('enable_application_live_view',
Copy link
Contributor

Choose a reason for hiding this comment

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

If this parameter has limited values, please add arg_type=get_enum_type(ConfigurationServiceGeneration) for the parameter and it is not necessay to specify the avaiable values manually in help.

Suggested change
c.argument('application_configuration_service_generation',
arg_group="Application Configuration Service",
type=ConfigurationServiceGeneration,
options_list=['--application-configuration-service-generation', '--acs-gen'],
validator=validate_acs_create,
help='(Enterprise Tier Only) Application Configuration Service Generation to enable. Allowed values are: ' + ', '.join(list(ConfigurationServiceGeneration)))
c.argument('enable_application_live_view',
c.argument('application_configuration_service_generation',
arg_group="Application Configuration Service",
arg_type=get_enum_type(ConfigurationServiceGeneration),
options_list=['--application-configuration-service-generation', '--acs-gen'],
validator=validate_acs_create,
help='(Enterprise Tier Only) Application Configuration Service Generation to enable.)
c.argument('enable_application_live_view',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! updated


for scope in ['create', 'update']:
with self.argument_context('spring application-configuration-service {}'.format(scope)) as c:
c.argument('generation', type=ConfigurationServiceGeneration, help='Generation. Allowed values are: ' + ', '.join(list(ConfigurationServiceGeneration)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

def validate_acs_create(namespace):
if namespace.application_configuration_service_generation is not None:
if namespace.enable_application_configuration_service is False:
raise ArgumentUsageError("--application_configuration_service_generation can only be set when enable application configuration service.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise ArgumentUsageError("--application_configuration_service_generation can only be set when enable application configuration service.")
raise ArgumentUsageError("--application-configuration-service-generation can only be set when enable application configuration service.")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

if namespace.generation is None:
return
if (namespace.generation not in list(ConfigurationServiceGeneration)):
raise InvalidArgumentValueError("Allowed generation are: " + ', '.join(list(ConfigurationServiceGeneration)))
Copy link
Contributor

Choose a reason for hiding this comment

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

After adding arg_type=get_enum_type() to the parameter, it is not necessary to valid the input values manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@yanzhudd
Copy link
Contributor

If there is new command added, please add test for the command.

if namespace.enable_application_accelerator:
raise ArgumentUsageError('--enable-application-accelerator {}'.format(suffix))
if namespace.application_configuration_service_generation:
raise ArgumentUsageError('--application_configuration_service_generation {}'.format(suffix))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise ArgumentUsageError('--application_configuration_service_generation {}'.format(suffix))
raise ArgumentUsageError('--application-configuration-service-generation {}'.format(suffix))

@yanzhudd
Copy link
Contributor

Do you want to release a new extension version? If so, please add the version to setup.py.


for scope in ['create', 'update']:
with self.argument_context('spring application-configuration-service {}'.format(scope)) as c:
c.argument('generation', arg_type=get_enum_type(ConfigurationServiceGeneration), help='Generation.')
Copy link
Contributor

Choose a reason for hiding this comment

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

help='Generation.'

Could you please provide the more specific and understandable help message?

@yanzhudd yanzhudd merged commit b8ae47b into Azure:main Jun 21, 2023
@azclibot
Copy link
Collaborator

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

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

Labels

AKS Auto-Assign Auto assign by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants