-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add central entities table with properties #4123
Conversation
One sqlc-generated file was forgotten, so I force-pushed to your branch to see if CI would be greener. |
SELECT id, 'artifact', name, project_id, provider_id, created_at FROM artifacts; | ||
|
||
INSERT INTO entities (id, entity_type, name, project_id, provider_id, created_at) | ||
SELECT id, 'pull_request', name, project_id, provider_id, created_at FROM pull_requests; |
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.
I'm not sure this is enough. Our infra first migrates the DB while the old code is running and then upgrades the minder code, which creates a race where an entry might be added to the old tables but never to the new ones.
What if we create a SQL trigger that starts writing to the new table on create and update instead, commit that first so that the existing code writes to the new tables and then copy the existing data? (I also wonder about the size of the data and if we'd create a huge transaction, but that's a matter of checking our SaaS data size)
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.
I think we should just choose a migration on the golang migration wrapper.
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.
+1 to doing the migration in the golang wrapper in a later PR after the tables are created and we start writing to them from the application code (but before we assume they are there on the read side).
I worry about doing SQL bulk inserts / updates leading to database locks, though I acknowledge that we haven't had those problems yet.
1f44ea9
to
1325b09
Compare
1325b09
to
48cdd99
Compare
48cdd99
to
13ac34e
Compare
Signed-off-by: Juan Antonio Osorio <[email protected]>
This way it runs every time minder updates Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
13ac34e
to
34a6d85
Compare
* Add central entities table with properties Signed-off-by: Juan Antonio Osorio <[email protected]> * Add entity population to migration command This way it runs every time minder updates Signed-off-by: Juan Antonio Osorio <[email protected]> * Create repositories on entities table as well Signed-off-by: Juan Antonio Osorio <[email protected]> --------- Signed-off-by: Juan Antonio Osorio <[email protected]>
* Add central entities table with properties Signed-off-by: Juan Antonio Osorio <[email protected]> * Add entity population to migration command This way it runs every time minder updates Signed-off-by: Juan Antonio Osorio <[email protected]> * Create repositories on entities table as well Signed-off-by: Juan Antonio Osorio <[email protected]> --------- Signed-off-by: Juan Antonio Osorio <[email protected]>
Summary
Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.
Fixes #(related issue)
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: