feat(web-analytics): Add a DB field cookieless_server_hash_mode to a team, and setting to change it behind a flag #27059
Conversation
…change it behind a flag
|
Size Change: +74 B (+0.01%) Total Size: 1.11 MB ℹ️ View Unchanged
|
| cookieless_server_hash_mode = models.SmallIntegerField( | ||
| default=CookielessServerHashMode.DISABLED, choices=CookielessServerHashMode.choices, null=True | ||
| ) |
There was a problem hiding this comment.
Why make it nullable? What Postgres version are we running?
If we are on PG 11+ we can make it non-nullable, and because there's a default it will automatically "backfill" it in a lightweight manner (i.e. it won't backfill, it computes it when returning the row)
There was a problem hiding this comment.
The problem with making it nullable is that we now need to handle DISABLED, STATEFUL, STATELESS and now null as well (which we aren't)
There was a problem hiding this comment.
SELECT version();
PostgreSQL 15.4 on aarch64-unknown-linux-gnu, compiled by aarch64-unknown-linux-gnu-gcc (GCC) 9.5.0, 64-
There was a problem hiding this comment.
So yes I should be able to use this if I can make django do the right thing
There was a problem hiding this comment.
Well DISABLED and null are just treated as the same atm (as both python and JS will treat them as equally falsy)
There was a problem hiding this comment.
Well DISABLED and null are just treated as the same atm (as both python and JS will treat them as equally falsy)
True, but is that a coincidence or a conscious choice? Similarly, this only true until we need to read this from Rust.
Anyway, this is going to work for now, and it is minor, but I still think that we should eventually move to a world where fields are not unconsciously nullable if they don't need to be
There was a problem hiding this comment.
I'm on it :)
Ah I forgot that I'd have to fight with django migrations. I'll merge this first and can do the django dance/fight later.
robbie-c
force-pushed
the
add/column-for-cookieless-mode
branch
from
5840342 to
e8c3cd0
Compare
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
Problem
Pulled this out of #25915 as it's a source of merge conflicts while it remains unmerged.
It's not actually used anywhere as of this PR, but it will be in a different PR.
This controls whether a team can send us cookieless events, and if they do, whether we use the stateless path or the stateful path.
See https://posthog.slack.com/archives/C06GG249PR6/p1734012638807509?thread_ts=1734012386.974969&cid=C06GG249PR6 for some more context but essentially.
See plugin-server/src/worker/ingestion/event-pipeline/cookielessServerHashStep.ts in https://github.com/PostHog/posthog/pull/25915/files for more.
Changes
Does this work well for both Cloud and self-hosted?
Yes
How did you test this code?
Updated plugin-server DB tests