Use Hikari rather than Commons DBCP in DataSource configuration examples

[rajadilipkolli]

From spring boot 2.x, the default DataSource library that comes with starter is Hikari, documentation is using commons-dbcp. To avoid confusion updating documentation to use HikariDataSource

[philwebb]

Thanks!

[snicoll]

@rajadilipkolli thanks for the PR but I am wondering what you see as confusing? This documentation you've updated is about configuring two datasources. These are manual configurations, and as such, don't really care about what the default in auto-configuration would be.

The documented examples also use two different datasources to show that's something possible if needed. I don't really see an interest at moving everything to Hikari.

[rajadilipkolli]

Hi @snicoll , when I copied the examples from documentation. It didn't work as import is not there then I realized that from starter only Hikari is there.

so, either we should change the example or update the documentation that we can use any choice of data source.

[snicoll]

. It didn't work as import is not there then I realized that from starter only Hikari is there.

Yes. This is an advanced configuration example of configuring multiple datasources. I don't believe that the starter changed from common dbcp to Hikari is the problem here.

update the documentation that we can use any choice of data source.

I don't understand what that means. The example already tries to show that you can use any choice of data source since it's doing exactly that.

You can't just copy/paste an example from the ref doc in "any" app and expect it to work. We could document that you need to add the relevant driver jar for the example to work but I personally am not sure that we need to go into that level of detail for an advanced example like this.

Let's see what the rest of the team thinks.

[mhalbritter]

I agree with @snicoll. This in an advanced example which shows the power of this approach, so that you can even configure two totally independent datasources (in this case Hikari and Commons DBCP).

We could maybe add a HINT: that explains that this example configures two unrelated connection pools from different libraries?

[wilkinsona]

I think we should accept the changes for the slice-test related examples as proposed. I don't think they need to use a custom type of DataSource to make their point.

As for the documentation on configuring an additional DataSource, arguably it is mixing two different things in the same section:

1. How to configure an additional DataSource
2. How to configure a DataSource of a non-default type

If you only care about 1, you don't need to see an example of 2. If you only care about 2, you don't need the complexity of it being an additional DataSource.

I think it might be clearer if the earlier "Configure a Custom DataSource" section covered using a custom type in some more detail. It already has an example using SomeDataSource as a bean but then shows using DataSourceBuilder with Hikari. If it showed how to use a custom type with DataSourceBuilder the later section on defining an additional datasource could use Hikari with a reference back to the "Configure a Custom DataSource" section that would now fully show how to use a pool other than Hikari.

[philwebb]

I agree with @wilkinsona, I think consistently using Hikari would be best.

[wilkinsona]

The return type needs to be changed to HikariDataSource as well.

[rajadilipkolli]

Hi @wilkinsona , I have updated PR to HikariDataSource but shouldn't it be DataSource ?

I feel DataSource is more accurate here.

[wilkinsona]

Thanks for your efforts here, @rajadilipkolli. Unfortunately, things seem to be getting further away from what we want. For example, the latest commits make some changes to some of the tests that shouldn't be necessary. They also unnecessarily change a configuration property prefix which means that the property names no longer align with those in the documented example.

I'm afraid we don't have time to guide you through the necessary changes step-by-step. To receive that level of guidance, please keep an eye out for an issue that's labelled as ideal for contribution. I'm going to close this one now but thanks anyway for your efforts here and bringing the potential for improvement to our attention. I've opened #43054 to tackle it.