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

Consider lowercase sslmode for easier JDBC URL adaption #486

Closed
nhajratw opened this issue Jan 31, 2022 · 3 comments
Closed

Consider lowercase sslmode for easier JDBC URL adaption #486

nhajratw opened this issue Jan 31, 2022 · 3 comments
Labels
status: ideal-for-contribution An issue that a contributor can help us with type: enhancement A general enhancement

Comments

@nhajratw
Copy link

Bug Report

Migrating from jdbc to r2dbc is confusing due to the jdbc parameter of sslmode and r2dbc requiring it to be cased differently as sslMode -- Ideally they should be the same, but at a minimum, i'd suggest a warning during URL parsing to indicate that the case differs.

Current Behavior

parameter sslmode does not work.

parameter sslMode works.

@nhajratw nhajratw added the status: waiting-for-triage An issue we've not yet triaged label Jan 31, 2022
@mp911de
Copy link
Collaborator

mp911de commented Feb 16, 2022

Parameter compatibility with JDBC was never a goal. However, this seems a small change to additionally consider lowercase sslmode. Feel free to submit a pull request, see

public static final Option<SSLMode> SSL_MODE = Option.valueOf("sslMode");
.

@mp911de mp911de added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 16, 2022
@mp911de mp911de changed the title sslmode parameter discrepancy from jdbc Consider lowercase sslmode for easier JDBC URL adaption Feb 16, 2022
@mp911de mp911de added the status: ideal-for-contribution An issue that a contributor can help us with label Feb 16, 2022
@govi20
Copy link
Contributor

govi20 commented Mar 12, 2022

@mp911de this change won't be backward compatible. shouldn't it be part of a major version release right?

@mp911de
Copy link
Collaborator

mp911de commented Mar 14, 2022

It should be. The approach is to introduce another constant and check it as fallback instead of replacing the existing constant for the option name.

ssttaarr pushed a commit to ssttaarr/r2dbc-postgresql that referenced this issue Apr 5, 2022
For easier JDBC URL adaption

[resolves pgjdbc#486]
mp911de pushed a commit that referenced this issue Apr 6, 2022
For easier JDBC URL adaption

[resolves #486]
mp911de added a commit that referenced this issue Apr 6, 2022
[#486][resolves #504]

Signed-off-by: Mark Paluch <[email protected]>
@mp911de mp911de closed this as completed in 312e483 Apr 6, 2022
mp911de added a commit that referenced this issue Apr 6, 2022
[#486][resolves #504]

Signed-off-by: Mark Paluch <[email protected]>
mp911de pushed a commit that referenced this issue Apr 6, 2022
For easier JDBC URL adaption

[resolves #486]
mp911de added a commit that referenced this issue Apr 6, 2022
[#486][resolves #504]

Signed-off-by: Mark Paluch <[email protected]>
@mp911de mp911de added this to the 0.8.13.RELEASE milestone Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ideal-for-contribution An issue that a contributor can help us with type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants