feat: limit grace period for annual customers to 7 days
Problem
Annual customers all have trust score of 15, which resulted in them never getting quota limited, which left a loophole to allow having unlimited free usage and/or over-ingestion in case of spikes.
Changes
Introduces a 7 day grace period for customers with trust score of 15
- We can continue using
never_drop_dataflag for VIPs or temporary exceptions - These changes will be combined with setting up alerts for sales team about customers where we're ignoring quota limiting due to trust score of 15 or
never_drop_dataso they can get in touch with them in parallel
For more context, see: https://posthog.slack.com/archives/C08ERRQT41E/p1748538038187629
Did you write or update any docs for this change?
- [x] No docs needed for this change
How did you test this code?
Updated tests
Approved, but I think the fact that
QUOTA_LIMIT_...constants andGRACE_PERIOD_DAYSvalues are not aligned is something we should revisit in the future.
Can you clarify what you mean by the values not being aligned?
Not saying it's wrong, just that is confusing at first sight that this is how it looks after being parsed:
GRACE_PERIOD_DAYS: dict[int, int] = {
3: 0,
7: 1,
10: 3,
15: 7,
}
It makes me think the trust score could be the same as the grace period days, but for some reason they aren't.
Ah got it, yeah I understand it might appear confusing. They shouldn't need to be aligned though IMO. My understanding is that trust scores (like membership types) are ints that are spread out so that we can in the future add more granular ones in between. They can serve many different purposes, related to other things, not just grace period days. The grace periods can also change over the time.
Maybe a way to reduce similar confusion would be to turn trust scores into enums (and rename it to map)?
This way it would look like:
GRACE_PERIOD_DAYS_MAP: dict[int, int] = {
CustomerTrustScore.LOW: QUOTA_LIMIT_LOW_TRUST_GRACE_PERIOD_DAYS,
CustomerTrustScore.MEDIUM: QUOTA_LIMIT_MEDIUM_TRUST_GRACE_PERIOD_DAYS,
CustomerTrustScore.MEDIUM_HIGH: QUOTA_LIMIT_MEDIUM_HIGH_TRUST_GRACE_PERIOD_DAYS,
}
(will keep it out of scope here since it's used in a lot more places)
Yeah I was going to suggest that, we shouldn't use integers, specially 1, 7, 15, that seem to have some kind of meaning, if they don't. We should replace them with enums. But nothing urgent.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.
Changed from 7 to 5 days (Simon felt even a day would be sufficient if they've explicitly set limits but let's start more generous and adjust later if it still makes sense)