-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
Feature 1665 k8s db auto update #1691
Conversation
mcVerStr = metacatVersion.getVersionString(); | ||
dbVersion = DBAdmin.getInstance().getDBVersion(); | ||
dbVerStr = dbVersion.getVersionString(); | ||
if (metacatVersion.compareTo(dbVersion) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to consider the scenario that the metacat version is less than the db version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I adopted the same logic as that used in the DB admin page, since the DBAdmin::upgradeDatabase() method calls DBAdmin.getUpdateScripts(), which iterates through the DB versions and figures out which updates need to be applied (and so it takes care of the cases where DB < metacat, and DB > Metacat).
I think that's a good approach, because the logic is in one place (DBAdmin.getUpdateScripts()) instead of three places, and if we ever decide to support downgrades, we can do that simply by changing DBAdmin, without changing the calling code.
What do you think?
Sounds good! Thank you!
…On Mon, Sep 18, 2023 at 1:53 PM Matthew B ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/edu/ucsb/nceas/metacat/startup/K8sAdminInitializer.java
<#1691 (comment)>:
> + * @throws ServletException if updates were unsuccessful
+ * @implNote package private to allow for unit testing
+ */
+ static void initK8sDBConfig() throws ServletException {
+
+ logMetacat.info("initK8sDBConfig(): checking for necessary database updates...");
+ MetacatVersion metacatVersion;
+ DBVersion dbVersion;
+ String mcVerStr = "NULL";
+ String dbVerStr = "NULL";
+ try {
+ metacatVersion = SystemUtil.getMetacatVersion();
+ mcVerStr = metacatVersion.getVersionString();
+ dbVersion = DBAdmin.getInstance().getDBVersion();
+ dbVerStr = dbVersion.getVersionString();
+ if (metacatVersion.compareTo(dbVersion) == 0) {
I adopted the same logic as that used in the DB admin page
<https://github.com/NCEAS/metacat/blob/372a79e1222b70f9001048d6ab45ae4d420b5bfb/lib/admin/metacat-configuration.jsp#L123>,
since the DBAdmin::upgradeDatabase() method calls
DBAdmin.getUpdateScripts()
<https://github.com/NCEAS/metacat/blob/d617d1480986ff89802ebe071de4fcbe88d2e504/src/edu/ucsb/nceas/metacat/admin/DBAdmin.java#L653>,
which iterates through the DB versions and figures out which updates need
to be applied (and so it takes care of the cases where DB < metacat, *and*
DB > Metacat).
I think that's a good approach, because the logic is in one place
(DBAdmin.getUpdateScripts()) instead of three places, and if we ever decide
to support downgrades, we can do that simply by changing DBAdmin, without
changing the calling code.
What do you think?
—
Reply to this email directly, view it on GitHub
<#1691 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB5QQDDFBCYFSBZC2MZIBELX3CYGLANCNFSM6AAAAAA42SE3I4>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
K8sAdminInitializer
) and did same for unit testscurl
method of logging into admin and running DB updates fromdocker-entrypoint.sh