Skip to content

Conversation

@p-bouchon
Copy link
Contributor

@p-bouchon p-bouchon commented Sep 20, 2023


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

Related command

'az containerapp job create'

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.

@azure-client-tools-bot-prd
Copy link

Hi @p-bouchon,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

@azure-client-tools-bot-prd
Copy link

Hi @p-bouchon,
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.

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Sep 20, 2023

⚠️Azure CLI Extensions Breaking Change Test
⚠️containerapp
rule cmd_name rule_message suggest_message
⚠️ 1008 - ParaPropAdd containerapp job create cmd containerapp job create update parameter max_executions: added property default=100
⚠️ 1008 - ParaPropAdd containerapp job create cmd containerapp job create update parameter parallelism: added property default=1
⚠️ 1008 - ParaPropAdd containerapp job create cmd containerapp job create update parameter polling_interval: added property default=30
⚠️ 1008 - ParaPropAdd containerapp job create cmd containerapp job create update parameter replica_completion_count: added property default=1
⚠️ 1008 - ParaPropAdd containerapp job create cmd containerapp job create update parameter replica_timeout: added property default=1800

@yonzhan
Copy link
Collaborator

yonzhan commented Sep 20, 2023

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

@yanzhudd
Copy link
Contributor

If you want to release a new version of the extension, please change the version in setup.py as well.

@Greedygre
Copy link
Contributor

Recommend PR title:
[containerapp] az containerapp job create: Add default values for properties

@p-bouchon p-bouchon changed the title Add container app job create default properties [containerapp] az containerapp job create: Add default values for properties Sep 26, 2023
@Greedygre
Copy link
Contributor

Greedygre commented Oct 6, 2023

Defaults defined here

Please do not paste the internal link.

Comment on lines 498 to 500
if self.get_argument_trigger_type() is None:
raise RequiredArgumentMissingError('Usage error: --trigger-type is required')
return super().validate_arguments()
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
if self.get_argument_trigger_type() is None:
raise RequiredArgumentMissingError('Usage error: --trigger-type is required')
return super().validate_arguments()
super().validate_arguments()
if self.get_argument_yaml() is None:
if self.get_argument_trigger_type() is None:
raise RequiredArgumentMissingError('Usage error: --trigger-type is required')

Copy link
Contributor

Choose a reason for hiding this comment

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

With --yaml, the --trigger-type is not required, right?

@Greedygre
Copy link
Contributor

Hi @yanzhudd @zhoxing-ms
Could you help to review this PR when you have chance? Thanks.

Comment on lines +501 to +507
c.argument('replica_completion_count', type=int, options_list=['--replica-completion-count', '--rcc'], help='Number of replicas that need to complete successfully for execution to succeed', default=1)
c.argument('replica_retry_limit', type=int, help='Maximum number of retries before the replica fails. Default: 0.', default=0)
c.argument('replica_timeout', type=int, help='Maximum number of seconds a replica can execute', default=1800)
c.argument('parallelism', type=int, help='Maximum number of replicas to run per execution', default=1)
c.argument('min_executions', type=int, help="Minimum number of job executions that are created for a trigger. Default: 0.", default=0)
c.argument('max_executions', type=int, help="Maximum number of job executions that are created for a trigger", default=100)
c.argument('polling_interval', type=int, help="Interval to check each event source in seconds.", default=30)
Copy link
Contributor

Choose a reason for hiding this comment

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

May I ask if these params are new added and have been tested?

Copy link
Contributor

@Greedygre Greedygre Oct 12, 2023

Choose a reason for hiding this comment

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

Yes, Tested in new added test test_containerapp_manualjob_defaults_e2e.

@yanzhudd yanzhudd merged commit edfe5b6 into Azure:main Oct 12, 2023
scrappywyrm pushed a commit to scrappywyrm/azure-cli-extensions that referenced this pull request Oct 16, 2023
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 ContainerApp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants