You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by bhaisaab <gi...@git.apache.org> on 2015/12/16 10:06:55 UTC

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

GitHub user bhaisaab opened a pull request:

    https://github.com/apache/cloudstack/pull/1247

    [4.7.0/master] DB Paths fixes: 4.5.3->4.7.0, 4.6.2->4.7.0

    - Adds missing upgrade path objects from 4.5.3 to 4.7.0
    - Adds an explicit upgrade path class from 4.6.2 to 4.7.0, that extends over
      the upgrade path from 4.6.1 to 4.7.0
    
    Notes:
    - The getUpgradableVersionRange() is not used to validate from/to versions which is why upgrading from 4.6.2 to 4.7.0 works without any errors when using the Upgrade461to470 for 4.6.2 version.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/shapeblue/cloudstack 47-dbpathsfix

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1247.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1247
    
----
commit 1ce3746d50db3f3e05857854b596e749760075fd
Author: Rohit Yadav <ro...@shapeblue.com>
Date:   2015-12-16T09:01:00Z

    engine/schema: Fix upgrade paths 4.5.3->4.7.0, 4.6.2->4.7.0
    
    - Adds missing upgrade path objects from 4.5.3 to 4.7.0
    - Adds an explicit upgrade path class from 4.6.2 to 4.7.0, that extends over
      the upgrade path from 4.6.1 to 4.7.0
    
    Signed-off-by: Rohit Yadav <ro...@shapeblue.com>

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1247#discussion_r48020299
  
    --- Diff: engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java ---
    @@ -242,13 +243,13 @@ public DatabaseUpgradeChecker() {
     
             _upgradeMap.put("4.5.2", new DbUpgrade[] {new Upgrade452to460(), new Upgrade460to461(), new Upgrade461to470()});
     
    -        _upgradeMap.put("4.5.3", new DbUpgrade[] {new Upgrade453to460()});
    +        _upgradeMap.put("4.5.3", new DbUpgrade[] {new Upgrade453to460(), new Upgrade460to461(), new Upgrade461to470()});
     
             _upgradeMap.put("4.6.0", new DbUpgrade[] {new Upgrade460to461(), new Upgrade461to470()});
     
             _upgradeMap.put("4.6.1", new DbUpgrade[] {new Upgrade461to470()});
     
    -        _upgradeMap.put("4.6.2", new DbUpgrade[] {new Upgrade461to470()});
    +        _upgradeMap.put("4.6.2", new DbUpgrade[] {new Upgrade462to470()});
    --- End diff --
    
    Please show what fails, it was tested before release. Let's not fix what isn't broken.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:

    https://github.com/apache/cloudstack/pull/1247#issuecomment-166814115
  
    Closing as the 4.5.3 issue has been fixed on 4.7/master. Will open a new PR for the 4.6.2 to 4.7.0 semantic upgrade path/class fix.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:

    https://github.com/apache/cloudstack/pull/1247#issuecomment-165062188
  
    @remibergsma yes, I've already shared this on dev@ that we need to fix this either in 4.7.0 or 4.7.1 :)
    Since, it's not a blocker for 4.7.0 per se we don't need to stop the voting but it would be ideal if we could fix the issue in 4.7.0


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:

    https://github.com/apache/cloudstack/pull/1247#issuecomment-165039772
  
    cc @remibergsma @DaanHoogland 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1247#discussion_r48020023
  
    --- Diff: engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java ---
    @@ -242,13 +243,13 @@ public DatabaseUpgradeChecker() {
     
             _upgradeMap.put("4.5.2", new DbUpgrade[] {new Upgrade452to460(), new Upgrade460to461(), new Upgrade461to470()});
     
    -        _upgradeMap.put("4.5.3", new DbUpgrade[] {new Upgrade453to460()});
    +        _upgradeMap.put("4.5.3", new DbUpgrade[] {new Upgrade453to460(), new Upgrade460to461(), new Upgrade461to470()});
     
             _upgradeMap.put("4.6.0", new DbUpgrade[] {new Upgrade460to461(), new Upgrade461to470()});
     
             _upgradeMap.put("4.6.1", new DbUpgrade[] {new Upgrade461to470()});
     
    -        _upgradeMap.put("4.6.2", new DbUpgrade[] {new Upgrade461to470()});
    +        _upgradeMap.put("4.6.2", new DbUpgrade[] {new Upgrade462to470()});
    --- End diff --
    
    I've explained the reason in detail on dev@. The upgrade path used for 4.6.2 version (source) is Upgrade461to470 that assumes from version 4.6.1 to 4.7.0 in getUpgradableVersionRange(). The fix tries to be consistent with the semantics followed in writing/using upgrade paths. Till upgrade process changes and new tools are introduced, I would like to be consistent with the pattern we've here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1247#discussion_r48008985
  
    --- Diff: engine/schema/src/com/cloud/upgrade/dao/Upgrade462to470.java ---
    @@ -0,0 +1,29 @@
    +// Licensed to the Apache Software Foundation (ASF) under one
    --- End diff --
    
    there is no need for this file


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1247#discussion_r48020663
  
    --- Diff: engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java ---
    @@ -242,13 +243,13 @@ public DatabaseUpgradeChecker() {
     
             _upgradeMap.put("4.5.2", new DbUpgrade[] {new Upgrade452to460(), new Upgrade460to461(), new Upgrade461to470()});
     
    -        _upgradeMap.put("4.5.3", new DbUpgrade[] {new Upgrade453to460()});
    +        _upgradeMap.put("4.5.3", new DbUpgrade[] {new Upgrade453to460(), new Upgrade460to461(), new Upgrade461to470()});
     
             _upgradeMap.put("4.6.0", new DbUpgrade[] {new Upgrade460to461(), new Upgrade461to470()});
     
             _upgradeMap.put("4.6.1", new DbUpgrade[] {new Upgrade461to470()});
     
    -        _upgradeMap.put("4.6.2", new DbUpgrade[] {new Upgrade461to470()});
    +        _upgradeMap.put("4.6.2", new DbUpgrade[] {new Upgrade462to470()});
    --- End diff --
    
    The PR tries to make the usage be consistent with the implementation and usage; see Upgrade453to460 class for example. The fix here is mainly for the 4.5.3 version, 4.6.2 upgrade path is semantically broken while it still works is another thing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab closed the pull request at:

    https://github.com/apache/cloudstack/pull/1247


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1247#discussion_r48008950
  
    --- Diff: engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java ---
    @@ -242,13 +243,13 @@ public DatabaseUpgradeChecker() {
     
             _upgradeMap.put("4.5.2", new DbUpgrade[] {new Upgrade452to460(), new Upgrade460to461(), new Upgrade461to470()});
     
    -        _upgradeMap.put("4.5.3", new DbUpgrade[] {new Upgrade453to460()});
    +        _upgradeMap.put("4.5.3", new DbUpgrade[] {new Upgrade453to460(), new Upgrade460to461(), new Upgrade461to470()});
     
             _upgradeMap.put("4.6.0", new DbUpgrade[] {new Upgrade460to461(), new Upgrade461to470()});
     
             _upgradeMap.put("4.6.1", new DbUpgrade[] {new Upgrade461to470()});
     
    -        _upgradeMap.put("4.6.2", new DbUpgrade[] {new Upgrade461to470()});
    +        _upgradeMap.put("4.6.2", new DbUpgrade[] {new Upgrade462to470()});
    --- End diff --
    
    there is no reason for this


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: [4.7.0/master] DB Paths fixes: 4.5.3->4.7...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/1247#issuecomment-165061200
  
    @bhaisaab Thanks for fixing / improving this. Will have a look and test it. Are you OK with handling this in 4.7.1 (due in say 2 weeks)? By that time 4.5.3 is also there.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---