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

Added support for Closeable in MySqlConnectionFactory #110

Closed
wants to merge 1 commit into from
Closed

Added support for Closeable in MySqlConnectionFactory #110

wants to merge 1 commit into from

Conversation

svats0001
Copy link
Contributor

@svats0001 svats0001 commented Apr 28, 2023

Hi @jchrys,

#66

I added support for the Closeable interface in MySqlConnectionFactory and I also removed the invocation of the deprecated getSocketTimeout() and replaced it with null in MySqlConnectionFactory. Let me know if there are any problems in the code.

Motivation: MySqlConnectionFactory could benefit from a way to release its resources through implementing the Closeable interface.

Modification: MySqlConnectionFactory now implements Closeable and contains the close() method that will close the underlying MySqlConnection resource. I also added a logger to add the close start/end progress to the log. I removed the configuration.getSocketTimeout() parameter from Client.connect in the from() method and replaced it with null.

Result: MySqlConnectionFactory now has a way for users to release its resources.

@jchrys
Copy link
Collaborator

jchrys commented Apr 28, 2023

hello, @svats0001.
I think it seems that it creates a new connection and returns close Mono. (it doesn't seem to affect the factory)

@svats0001
Copy link
Contributor Author

Oh I see. I'm not too sure what's required to close the ConnectionFactory. I'll try again later. Closing the PR.

@svats0001 svats0001 closed this May 2, 2023
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.

2 participants