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

Track organization artifacts cleanup #8418

Merged
merged 4 commits into from
Aug 26, 2021
Merged

Track organization artifacts cleanup #8418

merged 4 commits into from
Aug 26, 2021

Conversation

humitos
Copy link
Member

@humitos humitos commented Aug 17, 2021

Add an extra field on the Organization model to keep track if its artifacts
were already cleanup. This allows us to filter disabled organizations and run the
cleaning periodically.

The new field Organization.artifacts_cleaned is automatically set as False
when a new build happens for any Project under the organization and is set to
True when running a Django Admin action over the organization.

Add an extra field on the `Organization` model to keep track if its artificats
were already cleanup. This allows us to filter disabled organization and run the
cleaning periodically.

The new field `Organization.artifacts_cleaned` is automatically set as `False`
when a new build happens for any Project under the organization and is set to
`True` when running a Django Admin action over the organization.
@humitos humitos requested a review from a team August 17, 2021 15:16
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.

Need to change how we're setting this data.

readthedocs/organizations/signals.py Outdated Show resolved Hide resolved
@stsewd
Copy link
Member

stsewd commented Aug 23, 2021

If this is only taking into consideration if the organization has built versions, I think this can be a property instead, we already track if a version is built in the model

built = models.BooleanField(_('Built'), default=False)

@ericholscher
Copy link
Member

@stsewd I don't understand this comment. We are trying to solve a billing issue, not anything with the builders or Version state. Basically this PR: https://github.com/readthedocs/readthedocs-ops/pull/982/files

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 change looks simple enough 👍

@humitos humitos merged commit d8e99b7 into master Aug 26, 2021
@humitos humitos deleted the humitos/assets-cleanup branch August 26, 2021 09:03
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.

3 participants