You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Nate Cole <nc...@hortonworks.com> on 2014/11/04 22:55:05 UTC
Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/
-----------------------------------------------------------
Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
Bugs: AMBARI-8144
https://issues.apache.org/jira/browse/AMBARI-8144
Repository: ambari
Description
-------
Added stubs for /api/v1/clusters/c1/upgrades
This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/27590/diff/
Testing
-------
Only basic test added, more to come as functionality fills out.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 30:48.302s
[INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
[INFO] Final Memory: 29M/248M
[INFO] ------------------------------------------------------------------------
Thanks,
Nate Cole
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Alejandro Fernandez <af...@hortonworks.com>.
> On Nov. 5, 2014, 12:15 a.m., Alejandro Fernandez wrote:
> > Ship It!
Thanks for getting this out there so quickly.
- Alejandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59885
-----------------------------------------------------------
On Nov. 4, 2014, 9:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 9:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59885
-----------------------------------------------------------
Ship it!
Ship It!
- Alejandro Fernandez
On Nov. 4, 2014, 9:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 9:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Nate Cole <nc...@hortonworks.com>.
> On Nov. 4, 2014, 8:46 p.m., Tom Beerbower wrote:
> > Looks good Nate. So is this purely a placeholder API or is there a good chance that this will remain as the basis for the upgrade API going forward? I know that you had talked to John about some overlap with APIs that he may need.
> >
> > Also, the StaticallyInject notation is a clever way to deal with that mess. I appreciate it since I'm guilty of just following that bad pattern over and over again. I wonder though if we could have used @Singleton and the existing injection mechanism for most or all of these without introducing a new injection annotation. In other words, I think most of these could be singletons instead of non-singletons with static injection points. I don't think that we ever create more than one instance of any resource provider type, for example.
Thanks for reviewing! The intent is that this will be the API going forward, and we'll continue to build on it through the next month or so.
I was trying to come up with a way to use @Singleton, but didn't have the gumption to get very intrusive in order to make it work. Seems like the "real answer" would be to make a ResourceProvider available using an injector. Maybe that's the ultimate refactor that will take care of the madness.
- Nate
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59911
-----------------------------------------------------------
On Nov. 4, 2014, 4:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 4:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59911
-----------------------------------------------------------
Ship it!
Looks good Nate. So is this purely a placeholder API or is there a good chance that this will remain as the basis for the upgrade API going forward? I know that you had talked to John about some overlap with APIs that he may need.
Also, the StaticallyInject notation is a clever way to deal with that mess. I appreciate it since I'm guilty of just following that bad pattern over and over again. I wonder though if we could have used @Singleton and the existing injection mechanism for most or all of these without introducing a new injection annotation. In other words, I think most of these could be singletons instead of non-singletons with static injection points. I don't think that we ever create more than one instance of any resource provider type, for example.
- Tom Beerbower
On Nov. 4, 2014, 9:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 9:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Alejandro Fernandez <af...@hortonworks.com>.
> On Nov. 4, 2014, 11:22 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java, line 106
> > <https://reviews.apache.org/r/27590/diff/1/?file=749671#file749671line106>
> >
> > Hungarian anyone?
I prefer camelCase for Java, but ok with other notations as long as they are used consistently.
- Alejandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59846
-----------------------------------------------------------
On Nov. 4, 2014, 9:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 9:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Tom Beerbower <tb...@hortonworks.com>.
> On Nov. 4, 2014, 11:22 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java, line 106
> > <https://reviews.apache.org/r/27590/diff/1/?file=749671#file749671line106>
> >
> > Hungarian anyone?
>
> Alejandro Fernandez wrote:
> I prefer camelCase for Java, but ok with other notations as long as they are used consistently.
>
> Jonathan Hurley wrote:
> I've been coding with camelCase + hungarian for over a decade; it's hard to quit cold turkey from the hungarian. It would avoid cases like this where you'd have sUpgradeId (string) and iUpgradeId (int) and upgradeId (Object). But I think that there's just no way to change now as the codebase is too large.
I'll admit that in this case it might make the naming easier but I generally dislike hungarian because in some ways it makes the code less readable and harder to maintain.
While we are on the subject of readability... I don't understand the whole 'null == <variable>' convention. It had a purpose in C programming. It serves no purpose in Java that I know of except to make the code less readable. If I read the above code aloud in English, I would say "if upgrade id equals null", not the other way around.
Sorry for the rant.
- Tom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59846
-----------------------------------------------------------
On Nov. 4, 2014, 9:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 9:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Nate Cole <nc...@hortonworks.com>.
> On Nov. 4, 2014, 6:22 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java, line 83
> > <https://reviews.apache.org/r/27590/diff/1/?file=749671#file749671line83>
> >
> > Doesn't look like AMC is used here; maybe this can extend AbstractResourceProvider?
Patience! Still young, this code is.
- Nate
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59846
-----------------------------------------------------------
On Nov. 4, 2014, 4:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 4:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Nov. 4, 2014, 6:22 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java, line 106
> > <https://reviews.apache.org/r/27590/diff/1/?file=749671#file749671line106>
> >
> > Hungarian anyone?
>
> Alejandro Fernandez wrote:
> I prefer camelCase for Java, but ok with other notations as long as they are used consistently.
I've been coding with camelCase + hungarian for over a decade; it's hard to quit cold turkey from the hungarian. It would avoid cases like this where you'd have sUpgradeId (string) and iUpgradeId (int) and upgradeId (Object). But I think that there's just no way to change now as the codebase is too large.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59846
-----------------------------------------------------------
On Nov. 4, 2014, 4:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 4:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>
Re: Review Request 27590: Upgrade Execute: Create API endpoint for
upgrades and upgrade items
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27590/#review59846
-----------------------------------------------------------
Ship it!
Solid start so far.
ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java
<https://reviews.apache.org/r/27590/#comment101132>
Awesome! Can't wait to switch over the other providers to this model.
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java
<https://reviews.apache.org/r/27590/#comment101171>
Doesn't look like AMC is used here; maybe this can extend AbstractResourceProvider?
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java
<https://reviews.apache.org/r/27590/#comment101172>
Hungarian anyone?
- Jonathan Hurley
On Nov. 4, 2014, 4:55 p.m., Nate Cole wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2014, 4:55 p.m.)
>
>
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
>
>
> Bugs: AMBARI-8144
> https://issues.apache.org/jira/browse/AMBARI-8144
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Added stubs for /api/v1/clusters/c1/upgrades
>
> This test does not provide full DB connectivity. It's only meant to unblock other teams from having an endpoint to work with. There will be many many more changes over the course of the Upgrades initiative
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java f5de481
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java ab71ce2
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8
> ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 0974f6f
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java de63db5
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/27590/diff/
>
>
> Testing
> -------
>
> Only basic test added, more to come as functionality fills out.
>
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Nate Cole
>
>