You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Attila Magyar <am...@hortonworks.com> on 2017/02/13 13:48:22 UTC
Review Request 56599: ambari-server upgrade is not idempotent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/
-----------------------------------------------------------
Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
Bugs: AMBARI-19984
https://issues.apache.org/jira/browse/AMBARI-19984
Repository: ambari
Description
-------
Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
Diff: https://reviews.apache.org/r/56599/diff/
Testing
-------
Added new unittest that checks if adding RoleAuthorization is idempotent.
Tested manually:
- installed ambari 2.4
- upgraded to ambari 2.5
- set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
- upgraded to ambari 2.5 again
- started ambari-server checked the UI
Existing tests ran clealy
----------------------------------------------------------------------
Ran 465 tests in 15.977s
Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
Thanks,
Attila Magyar
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Attila Magyar <am...@hortonworks.com>.
> On Feb. 13, 2017, 3:49 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java, lines 243-251
> > <https://reviews.apache.org/r/56599/diff/1/?file=1631928#file1631928line243>
> >
> > Any reason you didn't use a Set here (especially since the contract only defines a Collection)?
>
> Attila Magyar wrote:
> The existing code uses ArrayList. I'm not sure if it will break anything if I change it to Set.
@jhurley
If we change it to set then we should remove the setter, and use addAuthorization() all over the place.
public void setAuthorizations(Collection<RoleAuthorizationEntity> authorizations) {
this.authorizations = authorizations;
}
The setAuthorizations() is used at one place in production code, and in 10 tests. But it will behave differently because it won't preserver the ordering. Or we can use LinkedHashSet to preserve the ordering too. What do you think?
- Attila
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165341
-----------------------------------------------------------
On Feb. 13, 2017, 1:48 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 1:48 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Feb. 13, 2017, 10:49 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java, lines 243-251
> > <https://reviews.apache.org/r/56599/diff/1/?file=1631928#file1631928line243>
> >
> > Any reason you didn't use a Set here (especially since the contract only defines a Collection)?
>
> Attila Magyar wrote:
> The existing code uses ArrayList. I'm not sure if it will break anything if I change it to Set.
>
> Attila Magyar wrote:
> @jhurley
>
> If we change it to set then we should remove the setter, and use addAuthorization() all over the place.
>
> public void setAuthorizations(Collection<RoleAuthorizationEntity> authorizations) {
> this.authorizations = authorizations;
> }
>
>
> The setAuthorizations() is used at one place in production code, and in 10 tests. But it will behave differently because it won't preserver the ordering. Or we can use LinkedHashSet to preserve the ordering too. What do you think?
A LinkedHashSet will retain order - beyond that, since you're doing the contains() check here which hash to iterate over the whole list, it seems to make sense to use a Set.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165341
-----------------------------------------------------------
On Feb. 13, 2017, 8:48 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 8:48 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Feb. 13, 2017, 10:49 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java, lines 243-251
> > <https://reviews.apache.org/r/56599/diff/1/?file=1631928#file1631928line243>
> >
> > Any reason you didn't use a Set here (especially since the contract only defines a Collection)?
>
> Attila Magyar wrote:
> The existing code uses ArrayList. I'm not sure if it will break anything if I change it to Set.
>
> Attila Magyar wrote:
> @jhurley
>
> If we change it to set then we should remove the setter, and use addAuthorization() all over the place.
>
> public void setAuthorizations(Collection<RoleAuthorizationEntity> authorizations) {
> this.authorizations = authorizations;
> }
>
>
> The setAuthorizations() is used at one place in production code, and in 10 tests. But it will behave differently because it won't preserver the ordering. Or we can use LinkedHashSet to preserve the ordering too. What do you think?
>
> Jonathan Hurley wrote:
> A LinkedHashSet will retain order - beyond that, since you're doing the contains() check here which hash to iterate over the whole list, it seems to make sense to use a Set.
I suggested using the LHS, but I don't think ReviewBoard published my comment. The reason I flagged this is because we're trying to enforce a data constraint using an if-statement (if !contains()) which could be error prone if this collection is modified elsewhere in the future. My thought was why not just use a collection that has this restriction built-in. If it's a pain to change for some reason (since callers use the setter method), then I'm OK leaving it. I thought it would be a relatively simply change.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165341
-----------------------------------------------------------
On Feb. 13, 2017, 8:48 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 8:48 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Attila Magyar <am...@hortonworks.com>.
> On Feb. 13, 2017, 3:49 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java, lines 243-251
> > <https://reviews.apache.org/r/56599/diff/1/?file=1631928#file1631928line243>
> >
> > Any reason you didn't use a Set here (especially since the contract only defines a Collection)?
The existing code uses ArrayList. I'm not sure if it will break anything if I change it to Set.
- Attila
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165341
-----------------------------------------------------------
On Feb. 13, 2017, 1:48 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 1:48 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165341
-----------------------------------------------------------
Fix it, then Ship it!
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java (lines 243 - 251)
<https://reviews.apache.org/r/56599/#comment237182>
Any reason you didn't use a Set here (especially since the contract only defines a Collection)?
- Jonathan Hurley
On Feb. 13, 2017, 8:48 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 8:48 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165335
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On Feb. 13, 2017, 8:48 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 8:48 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165364
-----------------------------------------------------------
Ship it!
Ship It!
- Alejandro Fernandez
On Feb. 13, 2017, 1:48 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 1:48 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165337
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Levas
On Feb. 13, 2017, 8:48 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 8:48 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Attila Magyar <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/
-----------------------------------------------------------
(Updated Feb. 14, 2017, 6:19 p.m.)
Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
Changes
-------
fixed imports
Bugs: AMBARI-19984
https://issues.apache.org/jira/browse/AMBARI-19984
Repository: ambari
Description
-------
Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java 8e69004
ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 1e68f9d
ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java 7fb8867
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 2836858
Diff: https://reviews.apache.org/r/56599/diff/
Testing
-------
Added new unittest that checks if adding RoleAuthorization is idempotent.
Tested manually:
- installed ambari 2.4
- upgraded to ambari 2.5
- set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
- upgraded to ambari 2.5 again
- started ambari-server checked the UI
Existing tests ran clealy
----------------------------------------------------------------------
Ran 465 tests in 15.977s
Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
Thanks,
Attila Magyar
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Attila Doroszlai <ad...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165534
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java (line 40)
<https://reviews.apache.org/r/56599/#comment237375>
[ERROR] /home/jenkins/jenkins-slave/workspace/Ambari-trunk-test-patch/ambari/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java:40: Wrong order for 'java.util.LinkedHashSet' import. [ImportOrder]
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java (line 391)
<https://reviews.apache.org/r/56599/#comment237376>
[ERROR] /home/jenkins/jenkins-slave/workspace/Ambari-trunk-test-patch/ambari/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java:38:8: Unused import - java.util.Collection. [UnusedImports]
- Attila Doroszlai
On Feb. 14, 2017, 3:38 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2017, 3:38 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java 8e69004
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 1e68f9d
> ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java 7fb8867
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java cee490b
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165529
-----------------------------------------------------------
Ship it!
Ship It!
- Robert Levas
On Feb. 14, 2017, 9:38 a.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2017, 9:38 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java 8e69004
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 1e68f9d
> ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java 7fb8867
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java cee490b
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Attila Magyar <am...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/
-----------------------------------------------------------
(Updated Feb. 14, 2017, 2:38 p.m.)
Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
Changes
-------
- replaced collection with set
- removed setter
Bugs: AMBARI-19984
https://issues.apache.org/jira/browse/AMBARI-19984
Repository: ambari
Description
-------
Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java 8e69004
ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 1e68f9d
ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java 7fb8867
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java cee490b
Diff: https://reviews.apache.org/r/56599/diff/
Testing
-------
Added new unittest that checks if adding RoleAuthorization is idempotent.
Tested manually:
- installed ambari 2.4
- upgraded to ambari 2.5
- set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
- upgraded to ambari 2.5 again
- started ambari-server checked the UI
Existing tests ran clealy
----------------------------------------------------------------------
Ran 465 tests in 15.977s
Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
Thanks,
Attila Magyar
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Laszlo Puskas <lp...@hortonworks.com>.
> On Feb. 13, 2017, 3:08 p.m., Laszlo Puskas wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java, line 1672
> > <https://reviews.apache.org/r/56599/diff/1/?file=1631931#file1631931line1672>
> >
> > Instead mocking tehe injector itself, add a module where instances are bound to mocks.
>
> Attila Magyar wrote:
> Tried it but didn't work, there are other dependencies that should be initialized.
Dropped as binding classes to instances cause the framework to interpret
- Laszlo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165330
-----------------------------------------------------------
On Feb. 13, 2017, 1:48 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 1:48 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Laszlo Puskas <lp...@hortonworks.com>.
> On Feb. 13, 2017, 3:08 p.m., Laszlo Puskas wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java, line 1672
> > <https://reviews.apache.org/r/56599/diff/1/?file=1631931#file1631931line1672>
> >
> > Instead mocking tehe injector itself, add a module where instances are bound to mocks.
>
> Attila Magyar wrote:
> Tried it but didn't work, there are other dependencies that should be initialized.
>
> Laszlo Puskas wrote:
> Dropped as binding classes to instances cause the framework to interpret
annotations and tries to inject all dependencies.
- Laszlo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165330
-----------------------------------------------------------
On Feb. 13, 2017, 1:48 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 1:48 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Attila Magyar <am...@hortonworks.com>.
> On Feb. 13, 2017, 3:08 p.m., Laszlo Puskas wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java, line 1672
> > <https://reviews.apache.org/r/56599/diff/1/?file=1631931#file1631931line1672>
> >
> > Instead mocking tehe injector itself, add a module where instances are bound to mocks.
Tried it but didn't work, there are other dependencies that should be initialized.
- Attila
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165330
-----------------------------------------------------------
On Feb. 13, 2017, 1:48 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 1:48 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>
Re: Review Request 56599: ambari-server upgrade is not idempotent
Posted by Laszlo Puskas <lp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56599/#review165330
-----------------------------------------------------------
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java (line 1666)
<https://reviews.apache.org/r/56599/#comment237174>
Instead mocking tehe injector itself, add a module where instances are bound to mocks.
- Laszlo Puskas
On Feb. 13, 2017, 1:48 p.m., Attila Magyar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56599/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 1:48 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Nate Cole, Robert Levas, and Sebastian Toader.
>
>
> Bugs: AMBARI-19984
> https://issues.apache.org/jira/browse/AMBARI-19984
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Running ambari-upgrade multiple times cause an exception due to violation of unique database constraint (the same RoleAuthorization can be added multiple times to admin permissions).
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java fb01cca
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java 4b33bcd
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java 0cd4f12f
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java 1c742ef
>
> Diff: https://reviews.apache.org/r/56599/diff/
>
>
> Testing
> -------
>
> Added new unittest that checks if adding RoleAuthorization is idempotent.
>
> Tested manually:
> - installed ambari 2.4
> - upgraded to ambari 2.5
> - set version back to 2.4 by running: update metainfo set metainfo_value = '2.4.0' where metainfo_key = 'version';
> - upgraded to ambari 2.5 again
> - started ambari-server checked the UI
>
>
> Existing tests ran clealy
>
> ----------------------------------------------------------------------
> Ran 465 tests in 15.977s
>
> Tests run: 4915, Failures: 0, Errors: 0, Skipped: 39
>
>
> Thanks,
>
> Attila Magyar
>
>