Skip to content

Limit concurrency per-organization #7489

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

Merged
merged 3 commits into from
Sep 21, 2020

Conversation

humitos
Copy link
Member

@humitos humitos commented Sep 17, 2020

Allow us to limit concurrency per-organization besides per-project. Both limits are compatible and can be used together, giving more priority to the limits per-project. This means we can limit a whole organization to 10 concurrent builds but a particular project from the organization could be limited to only 2.

NOTE: I think this is the first time we put logic code for Organization into the community site. I think it's fine now since we have all the models here. We could override the BuildQuerySetBase.concurrent method from corporate, but I think it's just more work without real benfits

Allow us to limit concurrency per-organization besides per-project. Both limits
are compatible and can be used together, giving more priority to the limits
per-project. This means we can limit a whole organization to 10 concurrent
builds but a particular project from the organization could be limited to only 2.
@humitos humitos requested a review from a team September 17, 2020 10:18
max_concurrent = project.max_concurrent_builds or settings.RTD_MAX_CONCURRENT_BUILDS
max_concurrent = (
project.max_concurrent_builds or
(organization.max_concurrent_builds if organization else 0) or
Copy link
Member

Choose a reason for hiding this comment

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

Do we want else 0 here? Will the 0 cause the or to move to the default? It seems a bit confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this may be a little tricky. Basically the 0 here works as False and makes the or to continue to the next value:

In [27]: None or 0 or 5
Out[27]: 5

This QuerySet takes into account the project limit, then the organization limit
and finally the default setting.

Using a QuerySet allows us to extend it from corporate and use
`Plan.max_concurrent_builds` field.
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

This looks good 👍

@humitos humitos merged commit d6452f0 into master Sep 21, 2020
@humitos humitos deleted the humitos/limit-concurrency-per-organization branch September 21, 2020 18:53
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.

2 participants