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.

Sign up for GitHub

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

Jump to bottom

Question: Should postgres-adapter & mysql-adapter implement the two-parameter acquireMigrationLock()? #456

Closed
bjornlll opened this issue May 1, 2023 · 2 comments
Closed

Question: Should postgres-adapter & mysql-adapter implement the two-parameter acquireMigrationLock()? #456

bjornlll opened this issue May 1, 2023 · 2 comments
Labels
built-in dialect Related to a built-in dialect mysql Related to MySQL postgres Related to PostgreSQL question Further information is requested

Comments

@bjornlll
Copy link

bjornlll commented May 1, 2023

The interface for createAdapter() requires a function acquireMigrationLock() that since a few versions back have a new signature:

  • Previous interface: acquireMigrationLock(db)
  • Current interface: acquireMigrationLock(db, options)

This is the commit that changed the interface ~3 months ago: fbcf3e3

Neither the MySQL adapter nor the Postgres adapter are implementing this interface.

Not sure if this is an issue, so feel free to close/ disregard, but flagging since I found it odd. Discovered when I was trying to track down an issue with SST where kysely-codegen doesn't seem to execute.
@bjornlll bjornlll changed the title Question: Should dialect/postgres/postgres-adapter implement the two parameter acquireMigrationLock() Question: Should dialect/postgres/postgres-adapter implement the two-parameter acquireMigrationLock() May 1, 2023
@bjornlll bjornlll changed the title Question: Should dialect/postgres/postgres-adapter implement the two-parameter acquireMigrationLock() Question: Should postgres-adapter & mysql-adapter implement the two-parameter acquireMigrationLock()? May 1, 2023
@koskimas
Copy link
Member

koskimas commented May 1, 2023

It's not an issue but we should add the parameter to the implementations to avoid confusion. The parameter won't be used for anything in those dialects though.

@igalklebanov igalklebanov added question Further information is requested built-in dialect Related to a built-in dialect postgres Related to PostgreSQL mysql Related to MySQL labels May 1, 2023
@bjornlll
Copy link
Author

bjornlll commented May 1, 2023

Got it. Thanks for getting back to me @koskimas

@savoiringfaire savoiringfaire mentioned this issue Jul 25, 2023
Open
savoiringfaire pushed a commit to savoiringfaire/storage that referenced this issue Jul 25, 2023
@marcus-kepler
Don't extend postgresql adapter
20d17be 
We can't extend the postgresql adapter until
kysely-org/kysely#456 is released, as it's not
currently fully implementing the interface. Instead copy the single
remaining member over for the time being.
savoiringfaire pushed a commit to savoiringfaire/storage that referenced this issue Aug 7, 2023
@marcus-kepler @savoiringfaire
Don't extend postgresql adapter
e5209bb 
We can't extend the postgresql adapter until
kysely-org/kysely#456 is released, as it's not
currently fully implementing the interface. Instead copy the single
remaining member over for the time being.
Gaspero pushed a commit to Gaspero/kysely that referenced this issue Oct 2, 2023
@koskimas @Gaspero
Closes kysely-org#456
bea76da
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
built-in dialect Related to a built-in dialect mysql Related to MySQL postgres Related to PostgreSQL question Further information is requested
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@koskimas @bjornlll @igalklebanov