Skip to content

Conversation

@amanda11
Copy link
Contributor

Add garbage collection for trace and rule_enforcement models.

@pull-request-size pull-request-size bot added the size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. label Mar 28, 2022
@amanda11 amanda11 marked this pull request as draft March 28, 2022 11:05
@amanda11 amanda11 added this to the 3.7.0 milestone Mar 28, 2022
@amanda11 amanda11 marked this pull request as ready for review March 28, 2022 11:54
@amanda11 amanda11 changed the title [WIP] Add garbage collection for trace and rule_enforcement models Add garbage collection for trace and rule_enforcement models Mar 28, 2022
@arm4b arm4b requested review from a team March 28, 2022 12:55
@cognifloyd cognifloyd merged commit 1041007 into master Mar 28, 2022
@cognifloyd cognifloyd deleted the rule_trace_housekeep branch March 28, 2022 14:07
"bin/st2-register-content",
"bin/st2-purge-executions",
"bin/st2-purge-trigger-instances",
"bin/st2-purge-trace",
Copy link
Member

Choose a reason for hiding this comment

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

For consistency it would be good to use plural form of the name here as well - e.g. purge-traces and purge-rule-enforcements.

cfg.IntOpt(
"rule_enforcement_ttl",
default=None,
help="Rule enforcements older than this value (days) will be automatically deleted.",
Copy link
Member

Choose a reason for hiding this comment

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

May be good to add something along the lines of "Defaults to None aka disabled by default." / "If set to None, it's disabled." to the end of the help strings.

help="Trigger instances older than this value (days) will be automatically deleted.",
),
cfg.IntOpt(
"rule_enforcement_ttl",
Copy link
Member

Choose a reason for hiding this comment

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

Same here, for consistency with the existing naming, the names should be plural.

@amanda11
Copy link
Contributor Author

@Kami @cognifloyd i will aim to push a pr to address kami's comments tomorrow, as they were raised after merge.
But will do before I do corresponding doc change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants