[GitHub] [geode] BenjaminPerryRoss commented on pull request #2918: GEODE-6102: add gfsh destroy data-source

2018-12-07 Thread GitHub
Agreed, we should switch to instance of to remove usage of == and to improve code reliability. [ Full content available at: https://github.com/apache/geode/pull/2918 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] BenjaminPerryRoss commented on pull request #2918: GEODE-6102: add gfsh destroy data-source

2018-12-06 Thread GitHub
The reason we elected to use a new class scoped data source class was so that in the future if which data sources are valid changes this test will still be comparing against an 'invalid' data source class. We originally tried to compare it against a Managed or XAPooled data source class but ran

[GitHub] [geode] BenjaminPerryRoss commented on pull request #2918: GEODE-6102: add gfsh destroy data-source

2018-11-29 Thread GitHub
We looked into changing the OK status from servers into ERROR to make it more clear when individual servers have issues, but this causes problems with the CliFunctionResult resolution in gfsh which disrupts behavior with the "ifExists" flag. Basically, if all the servers return the ERROR status

[GitHub] [geode] BenjaminPerryRoss commented on pull request #2918: GEODE-6102: add gfsh destroy data-source

2018-11-29 Thread GitHub
After looking over this, there is no code path which would allow us to attempt updating cluster config in the event the binding is not found within cluster config. @jchen21 The reason you might see those two messages together is in the event that there are no servers but cluster config is