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

max-tasks: Limit the number of tasks in a task file #2172

Closed
cidrblock opened this issue May 23, 2022 · 4 comments · Fixed by #3156
Closed

max-tasks: Limit the number of tasks in a task file #2172

cidrblock opened this issue May 23, 2022 · 4 comments · Fixed by #3156
Assignees
Labels
new rule A request for a new rule
Milestone

Comments

@cidrblock
Copy link
Contributor

Starting to pull in possible rule ideas from the community of practice: redhat-cop/automation-good-practices#64

Limiting the total number of tasks in a single task file would encourage the use of includes, possibly leading to more reusable discrete tasks.

This would also add to the readability of the task files as each would have a file name indicating the summary action it performs.

30 was thrown in here as an idea, it could be higher, but I think having it too low would move the complexity to the file system.

@cidrblock cidrblock added the new rule A request for a new rule label May 23, 2022
@ssbarnea ssbarnea changed the title Rule request: Limit the number of tasks in a task file max-tasks: Limit the number of tasks in a task file Jul 19, 2022
@rohitthakur2590
Copy link

I'll work on this one.

@MarkusTeufelberger
Copy link
Contributor

This is no longer recommended explicitly by the way: https://github.com/redhat-cop/automation-good-practices/tree/main/coding_style

It only reads

Break complex task files down into discrete parts.

Rationale
Task files that are very or and/or contain highly nested blocks are difficult to maintain. Breaking a large or complex task file into multiple discrete files makes it easier to read and understand what is being done in each part.

There is no specific number of tasks given.

@ssbarnea
Copy link
Member

@MarkusTeufelberger As part of the implementation for this rule, we will gather some data from all content published on public hub and find a suitable value that is very unlikely to be hit by normal usage. Also, the rule could be disabled by the user anyway.

This could be seen as related to pylint too-complex rule which is based on mccable cyclomatic complexity, something that does not really apply to ansible content. I suspect the number will be somewhere between 40-100.

Also the plan is to limit the number of tasks per block level (top level could also be considered a level, so you might have more tasks than the limit inside a single file without hitting the rule if they are nested.

Still, final implementation is still subject to discussions so any feedback is more than welcomed here. IMHO, we should be able to look at bad players once we can count the complexity and decide if the rule works or not based on rate of false positives.

If we discover that only ~50% of long tasks files should really be split in other files, we will not do the rule. But if more than 80% are and less than 20% are false positives, we might have a case for keeping it.

@ssbarnea
Copy link
Member

Lets try an first implementation checking for 100 tasks, user configurable and counted per file (not inside each branch).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new rule A request for a new rule
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants