eclipse-vertx / vertx-sql-client

High performance reactive SQL Client written in Java
Apache License 2.0
894 stars 200 forks source link

Mark scram-client in vertx-pg-client as required #1466

Closed jorsol closed 2 months ago

jorsol commented 2 months ago

Describe the feature

The scram-client dependency of the vertx-pg-client module is marked as optional, but since PostgreSQL 14+, the default value for password_encryption is scram-sha-256, so this dependency should not be marked as optional.

Use cases

To avoid recurring issues of failed authentication reports (due to the missing dependency), the dependency should be required. This improves the user experience when connecting to postgres and it does not have any downside.

Contribution

Who should implement this feature ? are you volunteering for implementing this feature or do you know that is able and willing implement this feature ?

Yes, I could volunteer if approved, it should be just removing the optional tag and updating the documentation.

vietj commented 2 months ago

actually we are more leaning toward making it optional https://github.com/eclipse-vertx/vertx-sql-client/pull/1461

vietj commented 2 months ago

at least in master, so I do have mixed feelings about this

jorsol commented 2 months ago

Postgres can use SCRAM auth since version 10+ and is used by default in Postgres 14+, in the end, more and more users will require the dependency to be available rather than be optional, at least is a good step to show an error message about the missing dependency.

vietj commented 2 months ago

@jorsol yes your words reasonnate, perhaps I should change my mind and make it the default instead of being optional.

The scram dependency seems to be a proper java module (explicit) and in that case I don't see this being an issue