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

Make validateConfiguration public to allow checking of validation query #1327

Merged

Conversation

jodygarnett
Copy link
Contributor

This method is used by GWC to check jdbc configuration files when loaded. Making the method public allows it to also be used by the GeoServer form.

@davidblasby
Copy link
Contributor

davidblasby commented Oct 28, 2024

Looks good, jody.

However, I thought the connection pool would normally validate the connection.

Also, if a specific validation query is required, why give the user the option of setting it? Perhaps this should be a UI change?

@jodygarnett
Copy link
Contributor Author

jodygarnett commented Oct 28, 2024

Yes, in this case I am wishing to provide form feedback from GeoServer incase they are missing a field.

Presently GeoServer DiskQuota screen just saves the file, and it is not until GWC tries to read it that the result is validated. GeoServer does not try connecting before saving the changed file!

… of validation query

This method is used by GWC to check jdbc configuration files when loaded. Making the method public allows it to also be used by the GeoServer form.
@jodygarnett jodygarnett force-pushed the disk-quota-check-connection branch from b2d8850 to e05edc0 Compare October 28, 2024 15:29
cp.setDriver("org.postgresql.Driver");
cp.setUrl("jdbc:postgresql:gttest");

cp.setValidationQuery("select 1 from DUAL");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think dual is only for Oracle.

pgsql# select 1 from dual;
ERROR:  relation "dual" does not exist

Copy link
Member

Choose a reason for hiding this comment

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

Yep

@jodygarnett jodygarnett merged commit 5b73090 into GeoWebCache:main Oct 28, 2024
7 checks passed
@jodygarnett
Copy link
Contributor Author

The backport label is not success

@@ -28,4 +29,7 @@ protected Properties createOfflineFixture() {
protected String getFixtureId() {
return "h2";
}

@Test
public void checkConnectionTest() {}
Copy link
Member

Choose a reason for hiding this comment

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

Don't think an empty test does much good... probably a leftover.

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

Successfully merging this pull request may close these issues.

3 participants