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

Db migration #49

Merged
merged 6 commits into from
Apr 8, 2024
Merged

Db migration #49

merged 6 commits into from
Apr 8, 2024

Conversation

mar235av
Copy link
Collaborator

@mar235av mar235av commented Apr 5, 2024

Updates to support migration to a GCP database.

  1. Updated the application context file with a new connection pool and added the SSL parameters.
  2. Updated the Database class, fixing one bad SQL query and two bad error handling sections.
  3. Updated the pom.xml, adding the new connection pool library and some minor version updates per Dependabot.

@mar235av mar235av requested a review from stea-uw April 5, 2024 01:26
Copy link
Contributor

@stea-uw stea-uw left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -79,8 +80,8 @@ public RelyingParty getRelyingPartyById(String id) throws RelyingPartyException
return rps.get(0);
}
else {
String errorMsg = String.format("error getting rp: %s, rps size = %s", id, rps.size());
log.debug(errorMsg);
String errorMsg = String.format("error getting rp: %s, rps size = %s", id, (rps == null) ? 0 : rps.size());
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional nit: Extract into a local variable.

It can help to minimize computations done in format statements, so that readers can focus on understanding the format string.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Going to ignore this for now. This error message is almost never used; I made the smallest update possible to get it to not NullPointerException.

@mar235av mar235av merged commit 3a59f84 into main Apr 8, 2024
1 check passed
@mar235av mar235av deleted the dbMigration branch April 8, 2024 23:34
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