feat(experimentation): whitelist server-side keys for warehouse ingestion#7865
feat(experimentation): whitelist server-side keys for warehouse ingestion#7865Zaimwa9 wants to merge 4 commits into
Conversation
…tion Sync an environment's client key and its active server-side keys to the ingestion whitelist when a warehouse connection is created, and remove them on delete. Each entry maps to the environment's client API key so the pipeline can attribute server-side-key events to it; server-side keys carry a Redis TTL matching their expiry.
Connect EnvironmentAPIKey post_save/post_delete signals so a server-side key's whitelisting tracks its validity, scoped to environments that have a warehouse connection.
Zaimwa9
requested review from
emyller and
gagantrivedi
and removed request for
a team
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
github-actions
Bot
added
api
Docker builds report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7865 +/- ##
==========================================
+ Coverage 98.59% 98.60% +0.01%
==========================================
Files 1472 1475 +3
Lines 57362 57809 +447
==========================================
+ Hits 56556 57003 +447
Misses 806 806 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
github-actions
Bot
added
docs
| environment_key: str, | ||
| expires_at: datetime | None = None, | ||
| ) -> None: | ||
| """Whitelist ``key`` for warehouse ingestion, mapping it to the canonical |
There was a problem hiding this comment.
nit: do we need this docstring?
| def delete_environment_key_from_ingestion(environment_api_key: str) -> None: | ||
| ingestion_sync_service.delete_environment_key(environment_api_key) | ||
| def remove_server_side_key_from_ingestion(key: str) -> None: | ||
| """Remove a deleted server-side key from the ingestion whitelist.""" |
There was a problem hiding this comment.
are we overdoing with the docstring?
| @@ -0,0 +1,32 @@ | |||
| from typing import Any | |||
There was a problem hiding this comment.
We're using hooks on the warehouse connection model but a signal here? Is there a reason we can't use hooks here too? Better to stick with one
| @register_task_handler() | ||
| def add_environment_key_to_ingestion(environment_api_key: str) -> None: | ||
| ingestion_sync_service.set_environment_key(environment_api_key) | ||
| def sync_environment_ingestion_keys(environment_id: int) -> None: |
There was a problem hiding this comment.
nit: sync implies that this is handling delete as well?
maybe this can just be write_environment_ingestion_keys considering we also have remove_environment_ingestion_keys
|
|
||
|
|
||
| @register_task_handler() | ||
| def reconcile_server_side_key_ingestion(environment_api_key_id: int) -> None: |
There was a problem hiding this comment.
This method name doesnt make much sense to me? This looks like just a singlular form of the above methods?
write_environment_ingestion_key ?
| @register_task_handler() | ||
| def delete_environment_key_from_ingestion(environment_api_key: str) -> None: | ||
| ingestion_sync_service.delete_environment_key(environment_api_key) | ||
| def remove_server_side_key_from_ingestion(key: str) -> None: |
There was a problem hiding this comment.
| def remove_server_side_key_from_ingestion(key: str) -> None: | |
| def remove_environment_ingestion_key(key: str) -> None: |
| instance: EnvironmentAPIKey, | ||
| **kwargs: Any, | ||
| ) -> None: | ||
| reconcile_server_side_key_ingestion.delay( |
There was a problem hiding this comment.
we also need to make sure we only trigger these if wearehouse connection is live
3 participants
docs/if required so people know about the feature.Changes
ser.…keys to the correctenvironment_keyexpires_atget a matching Redis TTLpost_save/post_deletesignals onEnvironmentAPIKeykeep the whitelist in sync on key rotation/deactivation/deletion (scoped to environments with a warehouse connection)EXISTStoGETHow did you test this code?
test_ingestion_sync_service.py,test_tasks.py,test_models.py,test_signals.py