Skip to content
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

fix(schema-engine): Ensure WS migrations can use shadow database #5021

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

SevInf
Copy link
Member

@SevInf SevInf commented Oct 16, 2024

Previousy, when creating shadow DB connection over websocket, we
connected to the same DB which broke in every migrate case except the
one that starts with clean migration history.

This PR ensures it works normally. Implementation is quite cursed
though: for WS we now allow to override db name via dbname query
string parameter. If set, we ignore dbname that we got from migration
server and use provided DB with the same username and password. Shadow
DB then uses this query string parameter to specify the url.

Close ORM-325

Previousy, when creating shadow DB connection over websocket, we
connected to the same DB which broke in every `migrate` case except the
one that starts with clean migration history.

This PR ensures it works normally. Implementation is quite cursed
though: for WS we now allow to override db name via `dbname` query
string parameter. If set, we ignore `dbname` that we got from migration
server and use provided DB with the same username and password. Shadow
DB then uses this query string parameter to specify the url.

Close ORM-325
@SevInf SevInf requested a review from a team as a code owner October 16, 2024 15:51
@SevInf SevInf requested review from jkomyno and removed request for a team October 16, 2024 15:51
@SevInf SevInf added this to the 5.22.0 milestone Oct 16, 2024
Copy link
Contributor

github-actions bot commented Oct 16, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.063MiB 2.063MiB 0.000B
Postgres (gzip) 824.206KiB 824.209KiB -3.000B
Mysql 2.034MiB 2.034MiB 0.000B
Mysql (gzip) 812.032KiB 812.033KiB -1.000B
Sqlite 1.929MiB 1.929MiB 0.000B
Sqlite (gzip) 770.617KiB 770.620KiB -3.000B

Copy link
Contributor

@jkomyno jkomyno left a comment

Choose a reason for hiding this comment

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

Reviewed while pairing. Let's ship it for EA. Shadow databases will eventually need to be supported on the Platform's side, so that this workaround is no longer needed.

@SevInf SevInf merged commit b9903dc into main Oct 17, 2024
385 of 387 checks passed
@SevInf SevInf deleted the fix/shadow-db-ws branch October 17, 2024 14:39
SevInf added a commit that referenced this pull request Oct 17, 2024
* fix(schema-engine): Ensure WS migrations can use shadow database

Previousy, when creating shadow DB connection over websocket, we
connected to the same DB which broke in every `migrate` case except the
one that starts with clean migration history.

This PR ensures it works normally. Implementation is quite cursed
though: for WS we now allow to override db name via `dbname` query
string parameter. If set, we ignore `dbname` that we got from migration
server and use provided DB with the same username and password. Shadow
DB then uses this query string parameter to specify the url.

Close ORM-325

* Clippy + rustfmt
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.

2 participants