Skip to content

Conversation

@SferaDev
Copy link
Member

@SferaDev SferaDev commented Feb 25, 2025

No description provided.

Copy link
Member

@tsg tsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the translation from SQL to drizzle for all the CRUD stuff (connections, integrations, clusters, etc.) is pretty straightforward and I like how the results look.

I'm unsure about the troubleshooting queries (the ones against targetdb), I kind of like having the raw SQL there so we can try them easier by copy&pasting.

I think the PR doesn't contain yet the migrations folder, right? I'm curious how that will look like.

Signed-off-by: Alexis Rico <[email protected]>
@SferaDev
Copy link
Member Author

SferaDev commented Feb 25, 2025

@tsg Type safety uncovered that you were mixing serial/number and string for id and we have /foo/add as in /foo/<id>. What would be your choice for the final schema (in a multi tenancy database) uuids, serials?

@tsg
Copy link
Member

tsg commented Feb 25, 2025

@SferaDev Ah, hmm, I guess it's either:

  • some string id, like UUIDv7
  • or a composite PK (tenant_id + serial)

Do you have a preference?

@SferaDev
Copy link
Member Author

I have a preference for uuids yeah but don't really mind if we go other way

@SferaDev SferaDev requested a review from tsg February 26, 2025 07:47
Signed-off-by: Alexis Rico <[email protected]>
Signed-off-by: Alexis Rico <[email protected]>
@SferaDev SferaDev added this pull request to the merge queue Feb 26, 2025
Merged via the queue into main with commit 1e570a1 Feb 26, 2025
4 checks passed
@SferaDev SferaDev mentioned this pull request Mar 19, 2025
@SferaDev SferaDev deleted the SferaDev/drizzle branch April 9, 2025 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants