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

Add PrestoConnection getProperties method to presto-jdbc #16329

Merged
merged 1 commit into from
Jul 21, 2021

Conversation

imillan
Copy link
Contributor

@imillan imillan commented Jun 24, 2021

Introduce a method to access PrestoConnection properties at runtime.
The usecase behind this request is described in issue #16320

Addresses issue #16320

Test plan

  • Run new test in TestJdbcConnection
  • mvn clean install -rf :presto-jdbc
  • Integration test used by loading the new driver and checking that method returns the properties used to open a presto connection
== RELEASE NOTES ==

JDBC changes
* Add method ``getConnectionProperties`` to PrestoConnection to allow for connection properties retrieval after a connection establishes. (:pr:`16329`)

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 24, 2021

CLA Signed

The committers are authorized under a signed CLA.

  • ✅ Ivan Millan (a652554aeb9ef08a707bca0ff673f6250da2fef7)

@imillan imillan force-pushed the prestoconnection-getproperties branch 2 times, most recently from 2797133 to a652554 Compare June 28, 2021 12:31
@imillan imillan requested a review from tdcmeehan June 30, 2021 14:56
@tdcmeehan tdcmeehan self-assigned this Jul 8, 2021
Copy link
Contributor

@tdcmeehan tdcmeehan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit. Also be sure to squash commits

@imillan imillan requested a review from tdcmeehan July 8, 2021 14:23
@imillan imillan force-pushed the prestoconnection-getproperties branch from 722cc57 to dcdd862 Compare July 8, 2021 17:26
@imillan
Copy link
Contributor Author

imillan commented Jul 20, 2021

@tdcmeehan I worked on the suggested nit and submitted a new request for review. thx

@tdcmeehan tdcmeehan merged commit 6c63b36 into prestodb:master Jul 21, 2021
@ajaygeorge
Copy link
Contributor

@imillan
Can you please format the Release notes as described by the guidelines here . The current one does not mention what type of change it is and has typos.

cc @tdcmeehan

@imillan
Copy link
Contributor Author

imillan commented Jul 30, 2021

@ajaygeorge
I reformatted the release notes. Let me know if something else is required.

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

Successfully merging this pull request may close these issues.

3 participants