[GitHub] [geode] dschneider-pivotal commented on pull request #4692: GEODE-7799: Distribute rebalance status to other locators

2020-02-18 Thread GitHub
done

[ Full content available at: https://github.com/apache/geode/pull/4692 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] dschneider-pivotal commented on pull request #4692: GEODE-7799: Distribute rebalance status to other locators

2020-02-18 Thread GitHub
We added "getFuture" to the ClusterManagementService. We thought this was 
cleaner than adding "getFuture" to ClusterManagementOperationResult because if 
we did that it would need to have a reference to the ClusterManagementService 
to repeatedly call "get".

[ Full content available at: https://github.com/apache/geode/pull/4692 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] dschneider-pivotal commented on pull request #4692: GEODE-7799: Distribute rebalance status to other locators

2020-02-14 Thread GitHub
I filed a geode ticket about moving those methods up to OperationResult.
The team talked and we decided it would be best to keep the impl class

[ Full content available at: https://github.com/apache/geode/pull/4692 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] dschneider-pivotal commented on pull request #4692: GEODE-7799: Distribute rebalance status to other locators

2020-02-14 Thread GitHub
I filed: https://issues.apache.org/jira/browse/GEODE-7803 for this

[ Full content available at: https://github.com/apache/geode/pull/4692 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] dschneider-pivotal commented on pull request #4692: GEODE-7799: Distribute rebalance status to other locators

2020-02-14 Thread GitHub
ps: I'm fine with some other name for the class that implements the 
RebalanceResult.
I also find it odd that RebalanceResult has "getSuccess()" and 
"getStatusMessage()" but its super interface "OperationResult" is empty. It 
seems like both of these would be reasonable things to expect from every 
OperationResult. 

[ Full content available at: https://github.com/apache/geode/pull/4692 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org


[GitHub] [geode] dschneider-pivotal commented on pull request #4692: GEODE-7799: Distribute rebalance status to other locators

2020-02-14 Thread GitHub
This PR did not introduce RebalanceResult nor RebalanceResultImpl.
I prefer keeping the current interface and having an internal implementation 
class. This prevents our external apis from having internal things exposed. For 
example, in this case, the external api does not need a constructor nor settors.
I think it would be good to have more of our external api contract defined by 
interfaces and try to minimize the number of classes in it.

[ Full content available at: https://github.com/apache/geode/pull/4692 ]
This message was relayed via gitbox.apache.org for 
notifications@geode.apache.org