You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2018/04/19 17:51:03 UTC

[GitHub] khos2ow commented on issue #2584: Enhance and cleanup DatabaseUpgradeChecker

khos2ow commented on issue #2584: Enhance and cleanup DatabaseUpgradeChecker
URL: https://github.com/apache/cloudstack/pull/2584#issuecomment-382824247
 
 
   @rafaelweingartner Adding a new schema upgrade path was really cumbersome and error-prone, because we needed to maintain a flat map of versions and their corresponding list of upgrade paths (`DbUpgrade`). Instead I'm proposing to have a logical hierarchy structure of versions:
   
   ```
   DatabaseVersionHierarchy.builder()
       .next("4.0.0"   , new Upgrade40to41())
       .next("4.0.1"   , new Upgrade40to41())
       .next("4.0.2"   , new Upgrade40to41())
       .next("4.1.0"   , new Upgrade410to420())
       .next("4.1.1"   , new Upgrade410to420())
       .next("4.2.0"   , new Upgrade420to421())
       ...
       .next("4.2.1"   , new Upgrade421to430())
       .next("4.9.3.0" , new Upgrade4930to41000())
       .next("4.10.0.0", new Upgrade41000to41100())
       .next("4.11.0.0", new Upgrade41100to41110())
       .build();
   ```
   
   With this approach, when we need to add a new version upgrade path, rather than add that (e.g. `Upgrade41200to41300`) in **dozens** of places, we only need to add it in *correct* place in the hierarchy. It will be even backward compatible, it means that we can inject a version in the middle of the hierarchy as well (e.g. between existing `4.9.3.0` AND `4.10.0.0`).
   
   As for your next question, originally I had it based on `master` but I figured it's good to have it on `4.11` forward, so I rebased it on 4.11 and waited for this exact discussion to mention what you mentioned. I even created another branch to target `master` with exact same fix. So we have two options, either apply this (with additional path) on `master` first, then apply this on `4.11` and when merge forward pretty much nothing will happen. Or apply this here and add extra path on master on another super-small PR.
   
   BTW there's a small logical problem which will be fixed in couple minutes! (that's why Travis fails now)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services