-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat(remap-self-managed): Self managed sources w/ exclusion ability #28747
base: master
Are you sure you want to change the base?
Conversation
📸 UI snapshots have been updated5 snapshot changes in total. 0 added, 5 modified, 0 deleted:
Triggered by this commit. |
Size Change: +27 B (0%) Total Size: 1.21 MB ℹ️ View Unchanged
|
{ | ||
loadTables: async () => { | ||
return await api.dataWarehouseTables | ||
.list({ exclusion_pattern: 'https://posthog-s3-datawarehouse' }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gilbert09 Let me know if this would be the best way before a more drastic change committing a column to the db...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR enhances data warehouse source management by implementing URL pattern exclusion functionality and improving table handling.
- Added
ExclusionFilter
in/posthog/warehouse/api/table.py
to filter out tables based on URL patterns - Updated
dataWarehouseTableLogic
to support URL prefix exclusion when loading tables - Added
created_at
field toDataWarehouseTable
interface intypes.ts
- Enhanced
DataWarehouseSelfManagedSourcesTable
with new columns and reorganized action buttons - Modified
api.ts
to accept query parameters for URL pattern exclusion indataWarehouseTables.list()
6 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile
frontend/src/scenes/data-warehouse/settings/DataWarehouseSelfManagedSourcesTable.tsx
Show resolved
Hide resolved
frontend/src/scenes/data-warehouse/settings/DataWarehouseSelfManagedSourcesTable.tsx
Show resolved
Hide resolved
frontend/src/scenes/data-warehouse/settings/dataWarehouseSettingsLogic.ts
Show resolved
Hide resolved
{ | ||
loadTables: async () => { | ||
return await api.dataWarehouseTables | ||
.list({ exclusion_url_pattern_prefix: 'https://posthog-s3-datawarehouse' }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I would shy away from this - we're just asking for trouble trying to do some string matching here. A table
model without a corresponding schema
model is how we identify these tables - is there a reason we're not running a migration to add some status/modifier/type to the table
table to differentiate these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok, I'll do it with a migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the self managed sources we do not seem to have other table linkings as far as I can tell, I did see that there is a created_by_id
column that is null in the case that our table was not created by a user.
…feat(re-map-self-managed)
📸 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
Added some views that don't pollute this list anymore
New and improved variant of #28457 where we now have the ability to exclude certain url prefixes on our api.
This is because there was an blunder in the first go round of this where we loaded all the tables since data warehouse tables are stored together with self managed sources.
Changes
url_pattern
prefixesDoes this work well for both Cloud and self-hosted?
Yes
How did you test this code?
Only see non-dw specific tables