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

MigrateDBConfig does not support pre-existing client and ensureDatabaseExists at the same time #74

Open
Tethik opened this issue Aug 17, 2021 · 4 comments

Comments

@Tethik
Copy link

Tethik commented Aug 17, 2021

I noticed the createDb was marked as deprecated, so I moved to refactor my code to support the newer syntax.

In my client I'd like to do the following:

  client = await createPostgresPool(); 
  const config: MigrateDBConfig = {
    client,
    ensureDatabaseExists: true,
    defaultDatabase: databaseName,
  };
  await migrate(config, "src/data/migrations");

However, the type defined for MigrateDBConfig does not allow for ensureDatabaseExists at the same time with an existing client:

export type MigrateDBConfig =
| (ConnectionParams & {
readonly database: string
} & EnsureDatabase)
| ClientParams

@ThomWright
Copy link
Owner

Yeah, honestly I'm not sure there's much value in supporting using existing clients - I can't even remember why I introduced it! I guess in case people want to use connection strings for the credentials or something?

Thanks for the issue. I'll see if I can find some time to look into supporting this combination of parameters.

@Tethik
Copy link
Author

Tethik commented Aug 17, 2021

In my case it means I don't have to write the same code twice for e.g. fetching the secrets and setting up the connection to the database (with properties such as pinned CA etc).

@Tethik
Copy link
Author

Tethik commented Aug 17, 2021

I can potentially try for a PR later this week too, but very hesitant to promise anything :)

@bruceharrison1984
Copy link

bruceharrison1984 commented Apr 28, 2022

The current shortfall of not allowing a custom client is this library doesn't support the ssl setting of the client. If removing the ability to use a custom client, the ssl setting should be made available through the client config object.

Another potential issue with removing it, I currently use the aws-xray-sdk to wrap the pg library so I get trace data. Removing the ability to pass in a client would make this impossible(or much mroe difficult).

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

No branches or pull requests

3 participants