[GitHub] [geode-native] mmartell commented on issue #407: GEODE-6054: build with vs 2017

2018-11-21 Thread GitHub
We were seeing a C4530 Warning: https://msdn.microsoft.com/en-us/library/2axwkyt4.aspx But I can't reproduce it. Trying a build without the /EHsc. Also, it seems like this would be the default for unmanaged but I can't find docs on that. On Wed, Nov 21, 2018 at 8:51 PM Michael Martell wrote: >

[GitHub] [geode-native] mmartell commented on issue #407: GEODE-6054: build with vs 2017

2018-11-21 Thread GitHub
I think some new Warnings as Errors shows up with the new (and likely stricter compiler). I will forward to you as soon as I finish this episode of Ozark. Not sure about using a different exception model between the managed and unmanaged sides. > On Nov 21, 2018, at 7:21 PM, Jacob Barrett

[GitHub] [geode-native] codecov-io commented on issue #406: GEODE-4728: Removed all usage of grid client

2018-11-21 Thread GitHub
# [Codecov](https://codecov.io/gh/apache/geode-native/pull/406?src=pr=h1) Report > Merging > [#406](https://codecov.io/gh/apache/geode-native/pull/406?src=pr=desc) > into > [develop](https://codecov.io/gh/apache/geode-native/commit/f19b552c9c9ec68c3f2abeb227b8b0e1e90012ea?src=pr=desc) > will

[GitHub] [geode-native] pivotal-jbarrett commented on issue #407: GEODE-6054: build with vs 2017

2018-11-21 Thread GitHub
Can you explain why you are changing the exception handling mode? How is it interacting with the .NET library since this exception mode is not supported for /clr? https://msdn.microsoft.com/en-us/library/ffkc918h.aspx > The following compiler options are not supported with /clr: >/EHsc and /EHs

[GitHub] [geode-native] pivotal-jbarrett commented on issue #407: GEODE-6054: build with vs 2017

2018-11-21 Thread GitHub
Can you explain why you are changing the exception handling mode? How is it interacting with the .NET library since this exception mode is not supported for /clr? > The following compiler options are not supported with /clr: >/EHsc and /EHs (/clr implies /EHa (see /EH (Exception Handling

[GitHub] [geode] jinmeiliao closed pull request #2889: GEODE-5943: cleanup vm before each test

2018-11-21 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2889 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on pull request #2889: GEODE-5943: cleanup vm before each test

2018-11-21 Thread GitHub
Will keep the ticket open [ Full content available at: https://github.com/apache/geode/pull/2889 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jinmeiliao commented on issue #2889: GEODE-5943: cleanup vm before each test

2018-11-21 Thread GitHub
failure not related to this PR [ Full content available at: https://github.com/apache/geode/pull/2889 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] upthewaterspout opened a new pull request #4: GEODE-6086: Looking for the output files in the correct location

2018-11-21 Thread GitBox
upthewaterspout opened a new pull request #4: GEODE-6086: Looking for the output files in the correct location URL: https://github.com/apache/geode-benchmarks/pull/4 Yardstick creates a subdirectory, so find the files in that directory

[GitHub] [geode-benchmarks] upthewaterspout opened pull request #4: GEODE-6086: Looking for the output files in the correct location

2018-11-21 Thread GitHub
Yardstick creates a subdirectory, so find the files in that directory [ Full content available at: https://github.com/apache/geode-benchmarks/pull/4 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on issue #407: GEODE-6054: build with vs 2017

2018-11-21 Thread GitHub
Thanks pivotal-jbarrett for a diligent review! All changes have been implemented. [ Full content available at: https://github.com/apache/geode-native/pull/407 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #407: GEODE-6054: build with vs 2017

2018-11-21 Thread GitHub
Moved this to root. [ Full content available at: https://github.com/apache/geode-native/pull/407 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] bschuchardt commented on pull request #2890: GEODE-6078: Allow client cache to use JSONFormatter in multi-user mode

2018-11-21 Thread GitHub
Since this is a public API it should have a javadoc [ Full content available at: https://github.com/apache/geode/pull/2890 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] bschuchardt commented on pull request #2890: GEODE-6078: Allow client cache to use JSONFormatter in multi-user mode

2018-11-21 Thread GitHub
Having two methods in a public API with the same spelling & only casing differences is a little odd. Also, this needs a javadoc. [ Full content available at: https://github.com/apache/geode/pull/2890 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2833: GEODE-5407: fix JMXMBeanReconnectDUnitTest timing

2018-11-21 Thread GitHub
+1 to an `await()` over this stopwatch implementation, unless I'm missing something important that pushes us to this over the other. If there's the compelling reason to not use `await()`, I'd put that in the updated javadoc. I also prefer an alias in the `await("Locators must agree on the

[GitHub] [geode] PurelyApplied commented on pull request #2833: GEODE-5407: fix JMXMBeanReconnectDUnitTest timing

2018-11-21 Thread GitHub
Does this Javadoc need updating? I don't see any reference to the `DEFAULT_TIMEOUT` that is claimed here, other than the actual `TIMEOUT` field incidentally also being 300 seconds. [ Full content available at: https://github.com/apache/geode/pull/2833 ] This message was relayed via

[GitHub] [geode] PurelyApplied commented on pull request #2833: GEODE-5407: fix JMXMBeanReconnectDUnitTest timing

2018-11-21 Thread GitHub
instead of while loop; can it be done using awaitility for each of the locator beans...and then compare them. ``` GeodeAwaitility.await().untilAsserted(() -> { getFederatedGemfireBeansFrom(locator2); finalL2Beans.clear();

[GitHub] [geode] PurelyApplied commented on pull request #2833: GEODE-5407: fix JMXMBeanReconnectDUnitTest timing

2018-11-21 Thread GitHub
instead of while loop; can it be done using awaitility for each of the locator beans...and then compare them. ``` GeodeAwaitility.await().untilAsserted(() -> { getFederatedGemfireBeansFrom(locator2); finalL2Beans.clear();

[GitHub] [geode] jchen21 closed pull request #2870: Update javadocs for data-source pool classes

2018-11-21 Thread GitHub
[ pull request closed by jchen21 ] [ Full content available at: https://github.com/apache/geode/pull/2870 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] bschuchardt commented on pull request #2885: GEODE-6034 Protobuf clients should not access or modify internal regions

2018-11-21 Thread GitHub
I see - thanks, I'll fix that [ Full content available at: https://github.com/apache/geode/pull/2885 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] smgoller commented on issue #3: Use a Java ssh server rather than docker for tests

2018-11-21 Thread GitBox
smgoller commented on issue #3: Use a Java ssh server rather than docker for tests URL: https://github.com/apache/geode-benchmarks/pull/3#issuecomment-440809053 Oh sweet! That makes more sense. ;) This is an automated

[GitHub] [geode-benchmarks] smgoller commented on issue #3: Use a Java ssh server rather than docker for tests

2018-11-21 Thread GitHub
Oh sweet! That makes more sense. ;) [ Full content available at: https://github.com/apache/geode-benchmarks/pull/3 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on issue #2892: GEODE-6079: Attempt to sync filesystem before searching for suspect s…

2018-11-21 Thread GitHub
Because we run tests in parallel, and because they all seem to be writing to the same suspect file, it may well be that either the DUnit run will timeout with the added time of the `sync()`, and/or the call will throw an `IOException` after timing out with frequency that makes this change

[GitHub] [geode] PurelyApplied closed pull request #2795: [Discuss] POMs with vs existing versioning.

2018-11-21 Thread GitHub
[ pull request closed by PurelyApplied ] [ Full content available at: https://github.com/apache/geode/pull/2795 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied closed pull request #2884: [Do not review] Add CleanupDUnitVMsRule before and after directly to DistributedRule …

2018-11-21 Thread GitHub
[ pull request closed by PurelyApplied ] [ Full content available at: https://github.com/apache/geode/pull/2884 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied opened pull request #2892: GEODE-6079: Attempt to sync filesystem before searching for suspect s…

2018-11-21 Thread GitHub
…trings. Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

[GitHub] [geode] galen-pivotal closed pull request #2841: GEODE-6039: deprecate DistributedTestCase and associated.

2018-11-21 Thread GitHub
[ pull request closed by galen-pivotal ] [ Full content available at: https://github.com/apache/geode/pull/2841 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] agingade commented on pull request #2885: GEODE-6034 Protobuf clients should not access or modify internal regions

2018-11-21 Thread GitHub
The question was about passing the cache instance to SecureFunctionServiceImpl(); line#57 since its not changed in this pr, the gui was displaying it in extreme left... [ Full content available at: https://github.com/apache/geode/pull/2885 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode] agingade commented on pull request #2885: GEODE-6034 Protobuf clients should not access or modify internal regions

2018-11-21 Thread GitHub
Works for me. [ Full content available at: https://github.com/apache/geode/pull/2885 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] codecov-io commented on issue #406: GEODE-4728: Removed all usage of grid client

2018-11-21 Thread GitHub
# [Codecov](https://codecov.io/gh/apache/geode-native/pull/406?src=pr=h1) Report > Merging > [#406](https://codecov.io/gh/apache/geode-native/pull/406?src=pr=desc) > into > [develop](https://codecov.io/gh/apache/geode-native/commit/f19b552c9c9ec68c3f2abeb227b8b0e1e90012ea?src=pr=desc) > will

[GitHub] [geode] mcmellawatt opened pull request #2891: GEODE-6052: Use constructor DI to avoid PowerMock in JMX tests

2018-11-21 Thread GitHub
Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [X] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [X] Has

[GitHub] [geode] agingade commented on pull request #2889: GEODE-5943: cleanup vm before each test

2018-11-21 Thread GitHub
I don't think we need to be using bounce to address the flakiness. We need to see where the calculation is getting wrong; if its with initial memory (before the test runs) or the memory usage/leak as the tests gets run and accordingly calculate the eviction. If its memory leak, need to address

[GitHub] [geode] nabarunnag closed pull request #2886: GEODE-5998: Using primitive type (LGTM fix)

2018-11-21 Thread GitHub
[ pull request closed by nabarunnag ] [ Full content available at: https://github.com/apache/geode/pull/2886 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-benchmarks] upthewaterspout opened pull request #3: Use a Java ssh server rather than docker for tests

2018-11-21 Thread GitHub
To test the ssh infrastructure, use apache sshd in the test to run a ssh server. This makes the tests less dependent on specific infrastructure and build magic. Users can now run the tests on their machines without having to set up passwordless ssh (Although actual benchmark runs will still

[GitHub] upthewaterspout closed pull request #2: Cleaning up benchmark output

2018-11-21 Thread GitBox
upthewaterspout closed pull request #2: Cleaning up benchmark output URL: https://github.com/apache/geode-benchmarks/pull/2 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign

[GitHub] [geode-benchmarks] upthewaterspout closed pull request #2: Cleaning up benchmark output

2018-11-21 Thread GitHub
[ pull request closed by upthewaterspout ] [ Full content available at: https://github.com/apache/geode-benchmarks/pull/2 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-benchmarks] upthewaterspout opened pull request #2: Cleaning up benchmark output

2018-11-21 Thread GitHub
Cleaning up the output of geode benchmarks by making sure that all directories are cleaned after the run, and configuring the per node and final output directory names. [ Full content available at: https://github.com/apache/geode-benchmarks/pull/2 ] This message was relayed via

[GitHub] upthewaterspout opened a new pull request #2: Cleaning up benchmark output

2018-11-21 Thread GitBox
upthewaterspout opened a new pull request #2: Cleaning up benchmark output URL: https://github.com/apache/geode-benchmarks/pull/2 Cleaning up the output of geode benchmarks by making sure that all directories are cleaned after the run, and configuring the per node and final output

[GitHub] [geode] jinmeiliao opened pull request #2890: GEODE-6078: Allow client cache to use JSONFormatter in multi-user mode

2018-11-21 Thread GitHub
* user does not have to provide credentials when creating the client cache in multi-user mode. Credentials are needed only when creating the view. * CacheProxy should have a reference to the JSONFormatter that is aware of the user attributes * clean up some tests and add more tests for coverage

[GitHub] [geode] jinmeiliao opened pull request #2889: GEODE-5943: cleanup vm before each test

2018-11-21 Thread GitHub
Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has

[GitHub] [geode] bschuchardt commented on pull request #2885: GEODE-6034 Protobuf clients should not access or modify internal regions

2018-11-21 Thread GitHub
I'm sorry but I don't understand this comment. The SecureCacheImpl needs to hold an InternalCacheForClientAccess, so this change seems necessary. [ Full content available at: https://github.com/apache/geode/pull/2885 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode] jdeppe-pivotal opened pull request #2888: GEODE-5971: Refactor Describe/List JNDI binding command to extend Gfs…

2018-11-21 Thread GitHub
…hCommand Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

[GitHub] [geode] bschuchardt commented on pull request #2885: GEODE-6034 Protobuf clients should not access or modify internal regions

2018-11-21 Thread GitHub
Yeah, I was surprised by that too. My PR isn't related to making the driver behave better. It's only about restricting access to internal regions. [ Full content available at: https://github.com/apache/geode/pull/2885 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode] jinmeiliao closed pull request #2863: GEODE-6064: redact the password in describeConfig command

2018-11-21 Thread GitHub
[ pull request closed by jinmeiliao ] [ Full content available at: https://github.com/apache/geode/pull/2863 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] bschuchardt commented on pull request #2885: GEODE-6034 Protobuf clients should not access or modify internal regions

2018-11-21 Thread GitHub
That approach of using setInternalRegion(), unfortunately, does not create a Region that is marked as an "internal region". I tried that and it doesn't work. I don't think it should be marked as PARTITIONED because PR admin regions aren't partitioned. [ Full content available at:

[GitHub] [geode] jdeppe-pivotal opened pull request #2887: GEODE-5971: Refactor server status/stop commands to return ResultModel

2018-11-21 Thread GitHub
Thank you for submitting a contribution to Apache Geode. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has

[GitHub] [geode-native] pivotal-jbarrett closed pull request #406: GEODE-4728: Removed all usage of grid client

2018-11-21 Thread GitHub
[ pull request closed by pivotal-jbarrett ] [ Full content available at: https://github.com/apache/geode-native/pull/406 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org