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

Limit concurrency per-organization #7489

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 Sep 17, 2020
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

@ericholscher ericholscher Sep 17, 2020

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

@humitos humitos Sep 18, 2020

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

readthedocs/organizations/models.py Show resolved Hide resolved
humitos added 2 commits Sep 21, 2020
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

This looks good 👍

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

None yet

2 participants