vapor / fluent-postgres-driver

🐘 PostgreSQL driver for Fluent.
MIT License
146 stars 53 forks source link

DatabaseConfigurationFactory build error in 2.7.0 #211

Closed rausnitz closed 1 year ago

rausnitz commented 1 year ago

Describe the bug

Starting in 2.7.0, the following code no longer builds for me:

let _ = DatabaseConfigurationFactory.postgres(
    configuration: PostgresConfiguration(url: "")!,
    maxConnectionsPerEventLoop: 1,
    connectionPoolTimeout: .seconds(30),
    sqlLogLevel: .info
)

The build error is:

cannot convert value of type 'PostgresConfiguration' to expected argument type 'SQLPostgresConfiguration'

To Reproduce

Paste in the code example above, using version 2.7.0 of this package.

Expected behavior

The code would build.

Environment

Tested on macOS and Linux.

Additional context

I haven't looked into the cause yet. I'll share a workaround as soon as I have a chance to make one, but ideally the code example above would still build after updating, and maybe a new patch could address that.

rausnitz commented 1 year ago

Looks like a simple change to address the issue: adding encoder and decoder arguments.

let _ = DatabaseConfigurationFactory.postgres(
    configuration: PostgresConfiguration(url: "")!,
    maxConnectionsPerEventLoop: 1,
    connectionPoolTimeout: .seconds(30),
    encoder: .init(),
    decoder: .init(),
    sqlLogLevel: .info
)

2.6.2 had default arguments for this function: https://github.com/vapor/fluent-postgres-driver/blob/2.6.2/Sources/FluentPostgresDriver/FluentPostgresConfiguration.swift#L78-L85.

The equivalent function in 2.7.0 doesn't have default arguments: https://github.com/vapor/fluent-postgres-driver/blob/main/Sources/FluentPostgresDriver/Deprecations/FluentPostgresConfiguration%2BDeprecated.swift#L57-L71.

I could make a PR to add the defaults back in, if that would be welcome.

0xTim commented 1 year ago

@rausnitz have you tried 2.7.1?

rausnitz commented 1 year ago

It has the same error.

gwynne commented 1 year ago

My fault, sorry 🤦‍♀️ I'll put up a PR to fix this shortly.