-
Notifications
You must be signed in to change notification settings - Fork 1.2k
schema: Add upgrade path from 4.21.0.0 to 4.22.0.0 #11469
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11469 +/- ##
============================================
- Coverage 17.35% 17.35% -0.01%
- Complexity 15230 15231 +1
============================================
Files 5886 5887 +1
Lines 525685 525724 +39
Branches 64159 64161 +2
============================================
+ Hits 91247 91252 +5
- Misses 424138 424172 +34
Partials 10300 10300
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
engine/schema/src/main/resources/META-INF/db/schema-42100to42200-cleanup.sql
Outdated
Show resolved
Hide resolved
engine/schema/src/main/resources/META-INF/db/schema-42100to42200.sql
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14673 |
@blueorangutan test |
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
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.
Pull Request Overview
This PR adds the database upgrade path from Apache CloudStack version 4.21.0.0 to 4.22.0.0. The implementation follows the standard CloudStack upgrade pattern by providing the necessary infrastructure for schema migration.
- Creates empty SQL migration scripts for the version upgrade
- Implements the Java upgrade class with standard boilerplate methods
- Registers the new upgrade path in the DatabaseUpgradeChecker chain
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
schema-42100to42200.sql |
Empty SQL migration script with Apache license header |
schema-42100to42200-cleanup.sql |
Empty SQL cleanup script with Apache license header |
Upgrade42100to42200.java |
Java upgrade implementation with standard methods and system VM template registration |
DatabaseUpgradeChecker.java |
Registration of the new upgrade path in the upgrade chain |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
} | ||
|
||
private void initSystemVmTemplateRegistration() { | ||
systemVmTemplateRegistration = new SystemVmTemplateRegistration(""); |
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.
The SystemVmTemplateRegistration constructor is being called with an empty string parameter. This may indicate missing configuration or could lead to unexpected behavior. Consider providing the appropriate parameter value or documenting why an empty string is intentional.
systemVmTemplateRegistration = new SystemVmTemplateRegistration(""); | |
// TODO: Replace "systemVmTemplateConfig" with the actual required configuration value. | |
systemVmTemplateRegistration = new SystemVmTemplateRegistration("systemVmTemplateConfig"); | |
// If an empty string is intentional, document the reason here. |
Copilot uses AI. Check for mistakes.
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.
thanks @weizhouapache LGTM
} | ||
|
||
@Override | ||
public void updateSystemVmTemplates(Connection conn) { |
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.
this is same in almost all the Upgrade* classes, can move this to interface as default implementation.
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.
yes, in most. some classes do not override it.
may need a new class
@Override | ||
public boolean supportsRollingUpgrade() { | ||
return false; | ||
} |
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.
add this as default implementation in interface
Description
This PR...
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?