[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-28 Thread karuturi
Github user karuturi commented on the issue: https://github.com/apache/cloudstack/pull/1768 Thanks everyone. merging this now. --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 [1768.network.results.txt](https://github.com/apache/cloudstack/files/805988/1768.network.results.txt)

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 @rhtyd I don't see how your concern is more true for the new code by @marcaurele then it is true for old code. cleanup is still going to be executed after the scripts, is it? only directly

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread rhtyd
Github user rhtyd commented on the issue: https://github.com/apache/cloudstack/pull/1768 @marcaurele I appreciate the cleanup, it should have been like this from the beginning but since we've the workflow to execute all the upgrades first and then the cleanup. In

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread karuturi
Github user karuturi commented on the issue: https://github.com/apache/cloudstack/pull/1768 Actually, BVT is not going to verify this as this is db upgrade related and travis would have tested it. I am merging this. --- If your project is set up for it, you can reply to this

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 @rhtyd I don't see how I could write such a test case. My point is exactly what you're saying, the upgrade would be different today for anyone who comes from an older version, which I don't

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread rhtyd
Github user rhtyd commented on the issue: https://github.com/apache/cloudstack/pull/1768 @marcaurele can we have a unit test to confirm that this does not break upgrade for older systems, also by changing the order for all older versions, the way someone would upgrade from say 4.3 to

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread blueorangutan
Github user blueorangutan commented on the issue: https://github.com/apache/cloudstack/pull/1768 @DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 @blueorangutan test --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-27 Thread karuturi
Github user karuturi commented on the issue: https://github.com/apache/cloudstack/pull/1768 @DaanHoogland Can you post test results? --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-25 Thread remibergsma
Github user remibergsma commented on the issue: https://github.com/apache/cloudstack/pull/1768 @marcaurele Nice improvement, thanks! Tested it and works as you described. LGTM. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-23 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 Forget my last comment, the files can remains as they are. --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-23 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 @blueorangutan test --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-23 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 I looked at the code inside `Upgrade481to490.java` and I cannot understand why the table alterations have been made inside the Java class instead of the SQL file. Because now we cannot move

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-23 Thread blueorangutan
Github user blueorangutan commented on the issue: https://github.com/apache/cloudstack/pull/1768 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-532 --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-23 Thread borisstoyanov
Github user borisstoyanov commented on the issue: https://github.com/apache/cloudstack/pull/1768 @blueorangutan package --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-23 Thread blueorangutan
Github user blueorangutan commented on the issue: https://github.com/apache/cloudstack/pull/1768 @borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. --- If your project is set up for it, you can reply to this email and have your

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 @BlueOrangUtan help --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 @BlueOrangUtan package --- 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

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 @syed the -cleanup scripts are for removing data that had been migrated, not for temporary tables. also a use case for those may be if things are done partly in the migrate script and

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 @syed I would prefer to move what's inside `schema-481to490-cleanup.sql` to the end of the file `schema-481to490.sql` as it would have been the way of processing during an update from 481 to

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread syed
Github user syed commented on the issue: https://github.com/apache/cloudstack/pull/1768 I agree with @DaanHoogland , as @marcaurele mentioned, the only file we need to worry about is `schema-481to490-cleanup.sql` the rest of them are either empty or change the configuration where the

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 As a reminder for `-cleanup` SQL scripts: > We rarely need cleanup scripts, only when some field/value is required during the data migration and has to be dropped later on.

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 @karuturi I think this should go in. Even being a bit of a risk, it will potentially reduce the support hours on upgrades. --- If your project is set up for it, you can reply to this

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 btw @marcaurele @rhtyd idempotent will only be possible if we encode it. It is not encoded in older upgrades. Unless we clean those upgrades up we will have some issues. --- If your

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-22 Thread DaanHoogland
Github user DaanHoogland commented on the issue: https://github.com/apache/cloudstack/pull/1768 LGTM but I hope you appreciate the concerns @marcaurele , on the other hand let's fix what breaks and make sure the 4.9 to 4.10 works with this. We can always advice to upgrade to 4.9

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-21 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 I'll try to make my point clearer with a better use case. Let say you were running version ACS 4.4.2 and wish to upgrade to 4.7.1. After installing the 4.7.1, when ACS starts for the first

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-21 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 @serg38 Not sure to understand what you mean with: > If I get it right with your proposed changes, upgrade scripts become obsolete since all the changes can be done in upgrade scripts.

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-21 Thread serg38
Github user serg38 commented on the issue: https://github.com/apache/cloudstack/pull/1768 @marcaurele I tend to agree with @rhtydthat it could break some of the upgrades. If I get it right with your proposed changes, upgrade scripts become obsolete since all the changes can be done

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2017-02-21 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 @rhtyd Changing the sequence is only idempotent if you have been upgrading to each new versions step by step. If you jumped other versions, then the path is different for each original

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2016-11-19 Thread rhtyd
Github user rhtyd commented on the issue: https://github.com/apache/cloudstack/pull/1768 This may break db upgrade, as the cleanup paths are run at the end. Can you prove if changing the sequence will be idempotent? Thanks. --- If your project is set up for it, you can reply to this

[GitHub] cloudstack issue #1768: CLOUDSTACK 9601: Upgrade: change logic for update pa...

2016-11-18 Thread marcaurele
Github user marcaurele commented on the issue: https://github.com/apache/cloudstack/pull/1768 The second commit can be removed if the cleanup is not wanted. --- 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