Skip to content
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

[#407] change default behavior of job_slice_count in job_launch #422

Merged

Conversation

adonisgarciac
Copy link
Contributor

@adonisgarciac adonisgarciac commented Nov 24, 2022

What does this PR do?

Change default behavior of job_slice_count.

job_slice_count is not set by default because is only allowed when ask_job_slice_count_on_launch is true but by default it is false.

I think it is better to set job_slice_count without default 1 and give the responsibility to user who has to know when set it up due to ask_job_slice_count_on_launch is true in the JT. Moreover, JT can have a default value which is 1 if user does not change it, so I think even although user will not send job_slice_count with ask_job_slice_count_on_launch=true, it will work.

Also it will work fine in AAP 2.2 and Tower.

Is there a relevant Issue open for this?

#407

Copy link
Contributor

@ivarmu ivarmu left a comment

Choose a reason for hiding this comment

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

LGTM.

Let's @djdanielsson and @sean-m-sullivan to review and approve.

@adonisgarciac adonisgarciac changed the title change default behavior of job_slice_count in job_launch [#407] change default behavior of job_slice_count in job_launch Nov 24, 2022
Copy link
Contributor

@silvinux silvinux left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@Tompage1994 Tompage1994 left a comment

Choose a reason for hiding this comment

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

Completely agree. We shoudln't be being opinionated here. The underlying module doesn't have a default so we should just be omitting this as we do with most other options.

@Tompage1994 Tompage1994 merged commit 0b88e25 into redhat-cop:devel Nov 24, 2022
@adonisgarciac adonisgarciac deleted the omit_default_job_slice_count branch January 26, 2023 07:48
przemkalit pushed a commit to przemkalit/aap_configuration that referenced this pull request Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants