Skip to content

Conversation

aaron-congo
Copy link
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@aaron-congo aaron-congo force-pushed the service-container-utility branch from 3e3ca46 to c465a00 Compare September 15, 2025 22:45
@aaron-congo aaron-congo changed the title Service container utility refactor: replace ConnectionService with ServiceUtility Sep 15, 2025
}

protected FullServicesContainer getNewServicesContainer() throws SQLException {
return ServiceUtility.getInstance().createServiceContainer(
Copy link
Contributor

@sergiyvamz sergiyvamz Oct 1, 2025

Choose a reason for hiding this comment

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

Just a suggestion. I've seen such piece of code at least twice. I think it makes sense to create a new method:
ServiceUtility.createNewFromSource(FullServicesContainer source, PluginService pluginService, Properties props)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it the same pluginService that is sitting in serviceContainer? Can we use
this.servicesContainer.getPluginService() instead?

private static final ReentrantLock initLock = new ReentrantLock();

private ServiceUtility() {
if (instance != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is it for?

}

public static ServiceUtility getInstance() {
if (instance != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This class is a simple stateless utility class. I think we can simplify logic a bit and use pre-initialized instance of ServiceUtility.

private static final ServiceUtility INSTANCE = new SericeUtility;

public ServiceUtility getInstance() { return INSTANCE; }

* {@link software.amazon.jdbc.util.ServiceUtility#createServiceContainer} followed by
* {@link PluginService#forceConnect} instead.
*/
@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

We're going to release 3.0 with some breaking changes. Do we want to add one more breaking change and completely remove this interface and implementation class?

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