Skip to content

Log URLs for all DataSources when H2 Console is auto-configured#28204

Closed
StitzL wants to merge 3 commits into
spring-projects:mainfrom
StitzL:2.5.x
Closed

Log URLs for all DataSources when H2 Console is auto-configured#28204
StitzL wants to merge 3 commits into
spring-projects:mainfrom
StitzL:2.5.x

Conversation

@StitzL

@StitzL StitzL commented Oct 6, 2021

Copy link
Copy Markdown
Contributor

Always log path of H2 console, then log details for each data source that offers a connection, in order, primary first.

Resolves #27881

@pivotal-cla

Copy link
Copy Markdown

@StitzL Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla

Copy link
Copy Markdown

@StitzL Thank you for signing the Contributor License Agreement!

@snicoll snicoll added this to the 2.6.x milestone Oct 6, 2021

@snicoll snicoll left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR.

We already have a test for the single datasource use case (see dataSourceUrlIsLoggedWhenAvailable). We'd need an additional test for the code that you've changed. Can you please look at that?

Also, this enhancement is not going to be backported so the base branch should be main (don't worry about it if you can't change it, we can take care of that as part of merging).

@snicoll snicoll changed the title Log URLs for all DataSources. Log URLs for all DataSources when H2 Console is auto-configured Oct 6, 2021
@StitzL

StitzL commented Oct 6, 2021

Copy link
Copy Markdown
Contributor Author

We already have a test for the single datasource use case (see dataSourceUrlIsLoggedWhenAvailable). We'd need an additional test for the code that you've changed. Can you please look at that?

Sure, I just have to figure out how to create a second datasource in that test scenario.

@snicoll

snicoll commented Oct 6, 2021

Copy link
Copy Markdown
Member

Sure, I just have to figure out how to create a second datasource in that test scenario.

Check MultiDataSourceUsingPrimaryConfiguration for example.

StitzL added 2 commits October 6, 2021 10:48
- Expect no log output at all when no DataSource offers a valid connection.
- Expect a single log line containing the path of the H2 console, followed by comma-separated URLs of each data source offering a connection.
Log path of H2 console, followed by comma-separated URLs of each data source that offers a connection, in order, primary first.
@StitzL

StitzL commented Oct 6, 2021

Copy link
Copy Markdown
Contributor Author

Condensed the log to a single entry with 4b0edea, which complicated the code.

Added tests for boundary conditions with 8f78983.

@StitzL StitzL requested a review from snicoll October 6, 2021 12:17
@snicoll snicoll changed the base branch from 2.5.x to main October 14, 2021 07:01
@snicoll snicoll self-assigned this Oct 14, 2021
@snicoll snicoll modified the milestones: 2.6.x, 2.6.0, 2.6.0-RC1 Oct 14, 2021
snicoll pushed a commit that referenced this pull request Oct 14, 2021
This commit logs the path of the H2 console, followed by the
comma-separated ordered URLs of each data source that offers a
connection.

See gh-28204
snicoll added a commit that referenced this pull request Oct 14, 2021
@snicoll snicoll closed this in 05ee8c6 Oct 14, 2021
@snicoll

snicoll commented Oct 14, 2021

Copy link
Copy Markdown
Member

@StitzL thank you for making your first contribution to Spring Boot.

@StitzL StitzL deleted the 2.5.x branch October 14, 2021 07:40
@StitzL

StitzL commented Oct 14, 2021

Copy link
Copy Markdown
Contributor Author

My pleasure! Thanks for the feedback and your work on this as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Log URLs for all DataSources when H2 Console is auto-configured

3 participants