You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Darren Shepherd <da...@gmail.com> on 2013/09/19 19:03:46 UTC
Review Request 14231: Don't check implementation verion of Object for
DatabaseUpgradeChecker
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14231/
-----------------------------------------------------------
Review request for cloudstack, Alex Huang and daan Hoogland.
Repository: cloudstack-git
Description
-------
Currently DatabaseUpgradeChecker determines the code version by doing this.getClass().getPackage().getImplementationVersion(). If it can't find the version it will eventually just give up and not do the database check. The problem currently is if it doesn't find the version, it will also check its parent's class version. The parent is java.lang.Object which will return the java version (for example 1.6.0_43). It doesn't seem like we would really want to ever try the JDK version as our code version, so this patch it to just effectively remove that check.
Diffs
-----
engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java f001bf7
Diff: https://reviews.apache.org/r/14231/diff/
Testing
-------
Thanks,
Darren Shepherd
Re: Review Request 14231: Don't check implementation verion of Object
for DatabaseUpgradeChecker
Posted by Daan Hoogland <da...@gmail.com>.
I know how to apply them, i don't want to.
On Wed, Sep 25, 2013 at 5:54 PM, Darren Shepherd <
darren.s.shepherd@gmail.com> wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14231/
>
> On September 25th, 2013, 12:34 p.m. UTC, *daan Hoogland* wrote:
>
> Ship It!
>
> On September 25th, 2013, 12:44 p.m. UTC, *daan Hoogland* wrote:
>
> Darren, this one is simple but next time submit a format created with 'git format-patch' please!?! I copied the lack of code, because it didn't 'git am' to the source.
>
> 9fb0a1a61992f93e2105d9a99d507d48a108b0c7 applied
>
> yeah, sorry. A lot of patches I have pending aren't made with format-patch. Will do for future. You just need to do "git apply --index <patch>" to apply a plain old patch. And then commit
>
>
> - Darren
>
> On September 19th, 2013, 5:03 p.m. UTC, Darren Shepherd wrote:
> Review request for cloudstack, Alex Huang and daan Hoogland.
> By Darren Shepherd.
>
> *Updated Sept. 19, 2013, 5:03 p.m.*
> *Repository: * cloudstack-git
> Description
>
> Currently DatabaseUpgradeChecker determines the code version by doing this.getClass().getPackage().getImplementationVersion(). If it can't find the version it will eventually just give up and not do the database check. The problem currently is if it doesn't find the version, it will also check its parent's class version. The parent is java.lang.Object which will return the java version (for example 1.6.0_43). It doesn't seem like we would really want to ever try the JDK version as our code version, so this patch it to just effectively remove that check.
>
>
> Diffs
>
> - engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java
> (f001bf7)
>
> View Diff <https://reviews.apache.org/r/14231/diff/>
>
Re: Review Request 14231: Don't check implementation verion of Object for
DatabaseUpgradeChecker
Posted by Darren Shepherd <da...@gmail.com>.
> On Sept. 25, 2013, 12:34 p.m., daan Hoogland wrote:
> > Ship It!
>
> daan Hoogland wrote:
> Darren, this one is simple but next time submit a format created with 'git format-patch' please!?! I copied the lack of code, because it didn't 'git am' to the source.
>
> 9fb0a1a61992f93e2105d9a99d507d48a108b0c7 applied
yeah, sorry. A lot of patches I have pending aren't made with format-patch. Will do for future. You just need to do "git apply --index <patch>" to apply a plain old patch. And then commit
- Darren
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14231/#review26373
-----------------------------------------------------------
On Sept. 19, 2013, 5:03 p.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14231/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2013, 5:03 p.m.)
>
>
> Review request for cloudstack, Alex Huang and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently DatabaseUpgradeChecker determines the code version by doing this.getClass().getPackage().getImplementationVersion(). If it can't find the version it will eventually just give up and not do the database check. The problem currently is if it doesn't find the version, it will also check its parent's class version. The parent is java.lang.Object which will return the java version (for example 1.6.0_43). It doesn't seem like we would really want to ever try the JDK version as our code version, so this patch it to just effectively remove that check.
>
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java f001bf7
>
> Diff: https://reviews.apache.org/r/14231/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14231: Don't check implementation verion of Object for
DatabaseUpgradeChecker
Posted by daan Hoogland <da...@gmail.com>.
> On Sept. 25, 2013, 12:34 p.m., daan Hoogland wrote:
> > Ship It!
Darren, this one is simple but next time submit a format created with 'git format-patch' please!?! I copied the lack of code, because it didn't 'git am' to the source.
9fb0a1a61992f93e2105d9a99d507d48a108b0c7 applied
- daan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14231/#review26373
-----------------------------------------------------------
On Sept. 19, 2013, 5:03 p.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14231/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2013, 5:03 p.m.)
>
>
> Review request for cloudstack, Alex Huang and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently DatabaseUpgradeChecker determines the code version by doing this.getClass().getPackage().getImplementationVersion(). If it can't find the version it will eventually just give up and not do the database check. The problem currently is if it doesn't find the version, it will also check its parent's class version. The parent is java.lang.Object which will return the java version (for example 1.6.0_43). It doesn't seem like we would really want to ever try the JDK version as our code version, so this patch it to just effectively remove that check.
>
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java f001bf7
>
> Diff: https://reviews.apache.org/r/14231/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14231: Don't check implementation verion of Object for
DatabaseUpgradeChecker
Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14231/#review26373
-----------------------------------------------------------
Ship it!
Ship It!
- daan Hoogland
On Sept. 19, 2013, 5:03 p.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14231/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2013, 5:03 p.m.)
>
>
> Review request for cloudstack, Alex Huang and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently DatabaseUpgradeChecker determines the code version by doing this.getClass().getPackage().getImplementationVersion(). If it can't find the version it will eventually just give up and not do the database check. The problem currently is if it doesn't find the version, it will also check its parent's class version. The parent is java.lang.Object which will return the java version (for example 1.6.0_43). It doesn't seem like we would really want to ever try the JDK version as our code version, so this patch it to just effectively remove that check.
>
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java f001bf7
>
> Diff: https://reviews.apache.org/r/14231/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>
Re: Review Request 14231: Don't check implementation verion of Object
for DatabaseUpgradeChecker
Posted by Daan Hoogland <da...@gmail.com>.
I am committing this if we don't get a reasonable explanation why we should
keep checking more then DatabaseUpgradeChecker itself.
rgards,
On Tue, Sep 24, 2013 at 6:26 PM, daan Hoogland <da...@gmail.com>wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14231/
> engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java<https://reviews.apache.org/r/14231/diff/1/?file=353986#file353986line387> (Diff
> revision 1)
>
> public void check() {
>
> 387
>
> currentVersion = this.getClass().getSuperclass().getPackage().getImplementationVersion();
>
> Could we check for an interface, allowing an ancestor between this and Object to supply the version? your argument makes sense, your solution seems drastic given that womeone took the trouble to write this code.
>
>
> - daan Hoogland
>
> On September 19th, 2013, 5:03 p.m. UTC, Darren Shepherd wrote:
> Review request for cloudstack, Alex Huang and daan Hoogland.
> By Darren Shepherd.
>
> *Updated Sept. 19, 2013, 5:03 p.m.*
> *Repository: * cloudstack-git
> Description
>
> Currently DatabaseUpgradeChecker determines the code version by doing this.getClass().getPackage().getImplementationVersion(). If it can't find the version it will eventually just give up and not do the database check. The problem currently is if it doesn't find the version, it will also check its parent's class version. The parent is java.lang.Object which will return the java version (for example 1.6.0_43). It doesn't seem like we would really want to ever try the JDK version as our code version, so this patch it to just effectively remove that check.
>
>
> Diffs
>
> - engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java
> (f001bf7)
>
> View Diff <https://reviews.apache.org/r/14231/diff/>
>
Re: Review Request 14231: Don't check implementation verion of Object for
DatabaseUpgradeChecker
Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14231/#review26355
-----------------------------------------------------------
engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java
<https://reviews.apache.org/r/14231/#comment51443>
Could we check for an interface, allowing an ancestor between this and Object to supply the version? your argument makes sense, your solution seems drastic given that womeone took the trouble to write this code.
- daan Hoogland
On Sept. 19, 2013, 5:03 p.m., Darren Shepherd wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14231/
> -----------------------------------------------------------
>
> (Updated Sept. 19, 2013, 5:03 p.m.)
>
>
> Review request for cloudstack, Alex Huang and daan Hoogland.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Currently DatabaseUpgradeChecker determines the code version by doing this.getClass().getPackage().getImplementationVersion(). If it can't find the version it will eventually just give up and not do the database check. The problem currently is if it doesn't find the version, it will also check its parent's class version. The parent is java.lang.Object which will return the java version (for example 1.6.0_43). It doesn't seem like we would really want to ever try the JDK version as our code version, so this patch it to just effectively remove that check.
>
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/upgrade/DatabaseUpgradeChecker.java f001bf7
>
> Diff: https://reviews.apache.org/r/14231/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Darren Shepherd
>
>