[GitHub] [geode-native] pivotal-jbarrett commented on pull request #347: GEODE-5707: CommitConflictException Test case.

2018-09-11 Thread GitHub
Would `EXPECT_THROW` not work in this case? [ Full content available at: https://github.com/apache/geode-native/pull/347 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett opened pull request #349: GEODE-5698: Fixes shared_ptr access.

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

[GitHub] [geode] rhoughton-pivot opened pull request #2457: GEODE-5600 - Run createVersionPropertiesFile on SHA change

2018-09-11 Thread GitHub
Found during testing, relating to GEODE-5694, remove the zip artifacts for better signing behavior Authored-by: Robert Houghton 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

[GitHub] [geode] rhoughton-pivot commented on issue #2457: GEODE-5600 - Run createVersionPropertiesFile on SHA change

2018-09-11 Thread GitHub
@pivotal-jbarrett @smgoller @nabarunnag This changes the signing behavior a little bit, as relates to ZIP artifacts. Please check it out. [ Full content available at: https://github.com/apache/geode/pull/2457 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] moleske commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-11 Thread GitHub
On GitHub this doesn't render as list making reading it more challenging. My understanding is that lettered lists are not valid markdown. I might suggest using either unordered lists, numbers, or using html (since that is all markdown is anyway) [ Full content available at: https

[GitHub] [geode] mcmellawatt opened pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-11 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? - [X] Has

[GitHub] [geode] gesterzhou opened pull request #2458: GEODE-5729: when DistributedCacheOperation needs 2 messages, should let

2018-09-11 Thread GitHub
the notifyOnly message to trigger callbacks And computeCompressedShort should not pack inhibitAllNotifications 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

[GitHub] [geode] nabarunnag closed pull request #2453: GEODE-5723: Don't publish to maven repo unless SNAPSHOT.

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

[GitHub] [geode] moleske commented on pull request #2456: Adding IntelliJ setup instructions to BUILDING.md

2018-09-11 Thread GitHub
This is a very Mac specific command, not sure that's what you intended [ Full content available at: https://github.com/apache/geode/pull/2456 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on pull request #2449: GEODE-4273: overhaul DiskRegionJUnitTest

2018-09-11 Thread GitHub
I copied that from the old test. Several tests use smaller literals and honestly, I just didn't realize that 10737418240l was equal to MAX_OPLOG_SIZE_IN_BYTES (is it?). I'll check if they're the same and change it. [ Full content available at: https://github.com/apache/geode/pull/2449 ] This

[GitHub] [geode] pdxrunner commented on pull request #2517: GEODE-2644: Cleanup logging classes

2018-09-27 Thread GitHub
Sorry, my original comment was too terse. My intent was as @sboorlagadda said. All part of the effort to remove platform dependencies like pathname separators, end of line conventions, classpath separators, etc. [ Full content available at: https://github.com/apache/geode/pull/2517 ] This

[GitHub] [geode] jdeppe-pivotal closed pull request #2513: GEODE-5212: Ensure that MergeLogs closes its InputStreams

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

[GitHub] [geode] jhuynh1 opened pull request #2523: GEODE-3967: Cleaning up of revert issues

2018-09-27 Thread GitHub
* The revert of previous code was missing test changes due to geode test structure changes 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

[GitHub] [geode] PurelyApplied commented on pull request #2509: GEODE-5777: Improvements for concourse_job_performance

2018-09-27 Thread GitHub
I am worried here if we have a run set that has a higher-indexed run that finishes before a lower-indexed run. I'm not entirely sure on how the `curl` request would return in that case, either. It's an edge-case to be sure, but this might be more stable as a filtered list comprehension. ```

[GitHub] [geode] PurelyApplied commented on pull request #2509: GEODE-5777: Improvements for concourse_job_performance

2018-09-27 Thread GitHub
PEP-8 is unhappy about double-newlines inside function definitions. [ Full content available at: https://github.com/apache/geode/pull/2509 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2509: GEODE-5777: Improvements for concourse_job_performance

2018-09-27 Thread GitHub
You can use `os.linesep` to be platform-agnostic in your newline padding. [ Full content available at: https://github.com/apache/geode/pull/2509 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on pull request #2509: GEODE-5777: Improvements for concourse_job_performance

2018-09-27 Thread GitHub
`os.linesep * 3` here, too. [ Full content available at: https://github.com/apache/geode/pull/2509 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on pull request #2517: GEODE-2644: Cleanup logging classes

2018-09-27 Thread GitHub
I think we should change every \n in the code base to be System.getProperty("line.separator") in one PR, but I'll go ahead and change this class. [ Full content available at: https://github.com/apache/geode/pull/2517 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode] dickcav closed pull request #2476: GEODE-3: small java 9 compatibility fixes

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

[GitHub] [geode] kirklund commented on pull request #2517: GEODE-2644: Cleanup logging classes

2018-09-27 Thread GitHub
@pdxrunner @sboorlagadda I'm actually not sure how to handle some of this logic: ```java if (endChar == '\n') { // trim off the \n since println will do it for us wordEnd--; if (wordEnd > 0 && target.charAt(wordEnd - 1) == '\r') { wordEnd--; } ``` I

[GitHub] [geode] dickcav closed pull request #2477: GEODE-3: Do not use @PostConstruct annotation which is not supported …

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

[GitHub] [geode] kirklund commented on pull request #2517: GEODE-2644: Cleanup logging classes

2018-09-27 Thread GitHub
I think we should have change every \n in the code base to be System.getProperty("line.separator") in one PR, but I'll go ahead and change this class. [ Full content available at: https://github.com/apache/geode/pull/2517 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode] jhuynh1 closed pull request #2523: GEODE-3967: Cleaning up of revert issues

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

[GitHub] [geode] pdxrunner commented on pull request #2517: GEODE-2644: Cleanup logging classes

2018-09-27 Thread GitHub
Sounds OK. [ Full content available at: https://github.com/apache/geode/pull/2517 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] sboorlagadda opened pull request #2524: GEODE-5788: On Windows, Ignore tests that bounces VMs

2018-09-27 Thread GitHub
bounce on windows is not graceful and causes the distributed system to start suspect processing. Ignoring these tests until GEODE-5787 is fixed. 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

[GitHub] [geode] kirklund opened pull request #2525: GEODE-2644: Use LINE_SEPARATOR in logging classes

2018-09-27 Thread GitHub
Use org.apache.commons.lang.SystemUtils.LINE_SEPARATOR instead of \n. These are all of the hardcoded \n I could find in a the internal.logging package (and sub-packages) except for the chars in LogWriterImpl#formatText. I left LogWriterImpl#formatText alone. This method is iterating over chars

[GitHub] [geode] kirklund closed pull request #2516: GEODE-5776: Use HexThreadIdPatternConverter for tid in logs

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

[GitHub] [geode] kirklund closed pull request #2517: GEODE-2644: Cleanup logging classes

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

[GitHub] [geode] kirklund commented on issue #2519: GEODE-2644: Cleanup logging tests

2018-09-27 Thread GitHub
I've resolved conflicts with develop. [ Full content available at: https://github.com/apache/geode/pull/2519 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on issue #2518: GEODE-2644: Cleanup config classes

2018-09-27 Thread GitHub
I removed the AtomicReference from LogConfig. Sai and I had intended to delete it but I missed it before. [ Full content available at: https://github.com/apache/geode/pull/2518 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] galen-pivotal opened pull request #2526: GEODE-5781: split repeatTest into separate tasks

2018-09-27 Thread GitHub
Because test classpaths can differ between source sets and modules, we should keep them separate, rather than combining them as `repeatTest` was doing before. This PR removes the original `repeatTest` task and replaces it with one repeat test task per source set. We tested the PR script by

[GitHub] [geode] smgoller opened pull request #2527: Src dist exclude fix

2018-09-27 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] smgoller closed pull request #2527: Src dist exclude fix

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

[GitHub] [geode] smgoller opened pull request #2528: Force inclusion of all ci/** files to work around exclude name collision

2018-09-27 Thread GitHub
src distribution excludes '**/out/**' which triggers on our concourse resource script entitled 'out'. This gets the script into the archive anyway Co-authored-by: Sean Goller Co-authored-by: Robert Houghton Thank you for submitting a contribution to Apache Geode. In order to streamline the

[GitHub] [geode] nabarunnag closed pull request #2528: Force inclusion of all ci/** files to work around exclude name collision

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

[GitHub] [geode] jdeppe-pivotal opened pull request #2529: GEODE-5792: ClientServerTransactionDUnitTest occasionally fails on Windows

2018-09-27 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] sboorlagadda opened pull request #2530: GEODE-5794: ignore JMXMBeanRecommentDUnitTest on windows

2018-09-27 Thread GitHub
These tests are failing and causing CI job to hang. So ignoring them until they are fixed. 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

[GitHub] [geode] jdeppe-pivotal opened pull request #2531: GEODE-5212: Also use the host's canonical (fully qualified) name as a SAN in the certificate

2018-09-27 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] PurelyApplied opened pull request #2532: [Discuss:] Eliminating transitive dependencies.

2018-09-27 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] kirklund opened pull request #2533: GEODE-2644: Cleanup LogService

2018-09-27 Thread GitHub
Two small commits that are part of GEODE-2644: ``` commit 46285f1ca33bcf2abd18382fd9996684e67be920 (HEAD -> GEODE-2644-Appenders-cleanup-LogService, kirklund-fork/GEODE-2644-Appenders-cleanup-LogService) Author: Kirk Lund Date: Thu Sep 27 16:04:25 2018 -0700 GEODE-2644: Cleanup

[GitHub] [geode] kirklund closed pull request #2518: GEODE-2644: Cleanup config classes

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

[GitHub] [geode] kirklund commented on issue #2519: GEODE-2644: Cleanup logging tests

2018-09-27 Thread GitHub
GfshParserAutoCompletionTest is failing in a very cryptic way. So far I don't know what the cause is. [ Full content available at: https://github.com/apache/geode/pull/2519 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] PurelyApplied commented on issue #2532: [Discuss:] Eliminating transitive dependencies.

2018-09-27 Thread GitHub
Please note that this PR is meant as a general discussion point. I fully expect many tests to fail, with actual dependencies hidden in reflection calls, etc. There will undoubtedly be additional iteration towards the "right" configuration of the source sets for each target. [ Full content

[GitHub] [geode] kirklund commented on issue #2519: GEODE-2644: Cleanup logging tests

2018-09-27 Thread GitHub
On develop, gfsh command "change loglevel --level" has these completions: ``` <[0. null - =ALL, 0. null - =DEBUG, 0. null - =ERROR, 0. null - =FATAL, 0. null - =INFO, 0. null - =OFF, 0. null - =TRACE, 0. null - =WARN]> ``` While on my branch, it has these completions:

[GitHub] [geode] kirklund opened pull request #2534: GEODE-2644: Improve GfshParserAutoCompletionTest assertions

2018-09-27 Thread GitHub
Improve assertions so that failures in this test can be diagnosed without using the IDE debugger. This test is failing on my branch for GEODE-2644 but the failure messages are very cryptic. This simple change makes the assertion messages easy to understand. Since this is valuable on develop, I'd

[GitHub] [geode] kirklund commented on issue #2534: GEODE-2644: Improve GfshParserAutoCompletionTest assertions

2018-09-27 Thread GitHub
I also fixed warnings and replaced \n with LINE_SEPARATOR. [ Full content available at: https://github.com/apache/geode/pull/2534 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal opened pull request #2535: GEODE-5780: LoggingThreadGroup is no longer used.

2018-09-27 Thread GitHub
Co-authored-by: @agingade LoggingUnhandledExceptionHandler creates a single handler that logs to a static logger. All geode threads should use this singleton. ThreadHelper should be used when creating a Thread. It will automatically set a LoggingUnhandledExceptionHandler on the thread.

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
Do we really need to add this whitespace here and the next file. If not then these two files would have no changes. [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] echobravopapa opened pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
[ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] echobravopapa commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
This is a partial conversion to using `CMAKE_DOTNET_TARGET_FRAMEWORK_VERSION` , there are some remaining csproj.in files belonging to `cli` test framework that will be addressed in https://issues.apache.org/jira/browse/GEODE-5818 until then we need both [ Full content available at:

[GitHub] [geode] pivotal-eshu commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

2018-10-05 Thread GitHub
Will address this concern with additional dunit test. [ Full content available at: https://github.com/apache/geode/pull/2571 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-eshu closed pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

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

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
The project GUID should not be in here as each time the project is generated the GUID will change. [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
Remove GUID [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
This is line should be deleted. [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

2018-10-05 Thread GitHub
it would be better to just use the "distributionManager" parameter. At some point we may find that this class no longer needs to keep a reference to the dm in an instance field. [ Full content available at: https://github.com/apache/geode/pull/2571 ] This message was relayed via

[GitHub] [geode] pivotal-eshu commented on pull request #2571: GEODE-5828: fixes replicates miss transaction commit.

2018-10-05 Thread GitHub
txTracker.commitProcessReceived method no longer need distribution manager anymore. [ Full content available at: https://github.com/apache/geode/pull/2571 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell closed pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
[ pull request closed by mmartell ] [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
@pivotal-jbarrett how's this look now? [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
done. [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
done. [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] mmartell commented on pull request #369: GEODE-5638: Organize/Cleanup the geode-native project structure

2018-10-05 Thread GitHub
done. [ Full content available at: https://github.com/apache/geode-native/pull/369 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett closed pull request #366: GEODE-5025: Uses templates to define Cacheable types

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

[GitHub] [geode] upthewaterspout opened pull request #2572: GEODE-5424: Changing all awaitility calls to use consistent timings

2018-10-05 Thread GitHub
We have a lot of Awaitility calls in our tests. Each test was picking its own timeout. That lead to some tests picking too small of a timeout and failing spuriously. With this change, all tests will use a new factory, GeodeAwaility.await(), rather than Awaitility.await(). This new factory sets a

[GitHub] [geode] rhoughton-pivot commented on pull request #2548: Better path resolution for srcDist exclusions

2018-10-08 Thread GitHub
Because 'should be' isn't the same as 'is', and it matches on regular files named 'build' and 'out' also. `out` is a required file name of a concourse-resource, and it was being excluded by this line. [ Full content available at: https://github.com/apache/geode/pull/2548 ] This message was

[GitHub] [geode] dschneider-pivotal closed pull request #2535: GEODE-5780: LoggingThreadGroup is no longer used.

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

[GitHub] [geode] jinmeiliao commented on issue #2567: GEODE-5820: upgrade jetty to fix jetty tests in java10

2018-10-05 Thread GitHub
we can change it to java 9+ [ Full content available at: https://github.com/apache/geode/pull/2567 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jdeppe-pivotal opened pull request #2577: GEODE-5794: fix failing JMXMBeanReconnectDUnitTest

2018-10-05 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] jdeppe-pivotal opened pull request #2578: GEODE-5829: Do not bind to both localhost and the actual address of the host

2018-10-05 Thread GitHub
- There is a subtle, low-level networking issue on Windows. See here https://stackoverflow.com/questions/52653506/udp-socket-binding-and-sending-behavior-on-windows-1709 and here https://serverfault.com/questions/934207/socket-binding-and-sending-on-different-addresses Thank you for

[GitHub] [geode] dhemery opened pull request #2579: GEODE-5501: Disconnect only current test's server

2018-10-05 Thread GitHub
* Disconnect would wait until there was a system, then disconnect it. Sometimes it would wait until the next test started a new instance, then immediately disconnect it. * Wait for the member to finish disconnecting before ending the test. * Remove the listener at the end of the test, so that it

[GitHub] [geode] kirklund closed pull request #2573: GEODE-2644: Cleanup logging classes and tests

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

[GitHub] [geode]

2018-10-05 Thread GitHub
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] rhoughton-pivot commented on pull request #2569: GEODE-5804: Improve checkPom task

2018-10-05 Thread GitHub
Why does the expected file need to exist in order to update/place. We could use this task to generate the baseline for a newly-minted module. [ Full content available at: https://github.com/apache/geode/pull/2569 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode] PurelyApplied commented on pull request #2569: GEODE-5804: Improve checkPom task

2018-10-05 Thread GitHub
I suppose it doesn't! [ Full content available at: https://github.com/apache/geode/pull/2569 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] jdeppe-pivotal opened pull request #2574: GEODE-5212: Also use the host's canonical (fully qualified) name as a…

2018-10-05 Thread GitHub
… SAN in the certificate 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

[GitHub] [geode] PurelyApplied commented on issue #2569: GEODE-5804: Improve checkPom task

2018-10-05 Thread GitHub
It seems like the actual `checkPom` task is only examining `` blocks and not any other data. So, on the one hand, the existing behavior was "copy the actual pom to the expected pom" is what's happening, but a developer will then need to examine which difference should actually be added to

[GitHub] [geode] kirklund opened pull request #2573: GEODE-2644: Cleanup logging classes and tests

2018-10-05 Thread GitHub
This is divided up into smaller commits: GEODE-2644: Add LogWriterLevel enum and test GEODE-2644: Cleanup LogLevel and expand LogLevelTest GEODE-2644: Cleanup custom log4j2 config logging tests GEODE-2644: Cleanup Marker Filter logging tests GEODE-2644: Cleanup logging tests GEODE-2644: Cleanup

[GitHub] [geode] jchen21 closed pull request #2556: GEODE-5711: Gfsh prompt for JNDI username and password

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

[GitHub] [geode] PurelyApplied commented on issue #2569: GEODE-5804: Improve checkPom task

2018-10-05 Thread GitHub
I had been poking at it to make sure it's still good. I was getting some strange behavior where `./gradlew uEP` updates the assembly pom with ``` diff --git a/geode-assembly/src/test/resources/expected-pom.xml b/geode-assembly/src/test/resources/expected-pom.xml index 3f8abf5e57..b22bc8f393

[GitHub] [geode] jinmeiliao closed pull request #2567: GEODE-5820: upgrade jetty to fix jetty tests in java10

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

[GitHub] [geode] jinmeiliao opened pull request #2576: GEODE-5821: exclude geode 1.4 when testing with java 9+

2018-10-05 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 closed pull request #2562: GEODE-5686: Remove LocalizedStrings

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

[GitHub] [geode] dschneider-pivotal opened pull request #2575: refactor RegionMapDestroy

2018-10-05 Thread GitHub
This branch started out as work on unit test coverage for RegionMapDestroy. That test coverage has been checked in with a different pull request. This pull request has the refactoring that was done while writing the unit tests. Thank you for submitting a contribution to Apache Geode. In order

[GitHub] [geode] pivotal-jbarrett commented on pull request #2548: Better path resolution for srcDist exclusions

2018-10-08 Thread GitHub
Well that’s just downright a big in Gradle then. Any open tickets on the issue? [ Full content available at: https://github.com/apache/geode/pull/2548 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on pull request #2548: Better path resolution for srcDist exclusions

2018-10-08 Thread GitHub
While I am not a fan of complicated builds I would rather keep the hacks for bugs closer to the source. In this case the bug is in Gradle so let's keep the fix in Gradle. Changing the Docker build to rename files to match the Concourse API would be making another build processes, removed from

[GitHub] [geode] metatype commented on pull request #2548: Better path resolution for srcDist exclusions

2018-10-08 Thread GitHub
Eh, I would rather find a way to keep the build file simple. Instead of writing gradle code to work around this, how `bout changing the file names...? [ Full content available at: https://github.com/apache/geode/pull/2548 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode] jdeppe-pivotal closed pull request #2577: GEODE-5794: fix failing JMXMBeanReconnectDUnitTest

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

[GitHub] [geode] jdeppe-pivotal closed pull request #2574: GEODE-5212: Also use the host's canonical (fully qualified) name as a…

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

[GitHub] [geode] upthewaterspout commented on issue #2548: Better path resolution for srcDist exclusions

2018-10-08 Thread GitHub
I'm fine with this change, but I do wonder if there is simpler way to do this in gradle - I think the exclude can take a closure, so maybe something like this would work? ``` exclude {it.path.contains('/out/') && !it.path.contains('/ci'} ```

[GitHub] [geode] balesh2 commented on pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

2018-10-11 Thread GitHub
You could use "untilAsserted" here. You would have to use a soft assertion if you wanted to see all three of those values every time, but even if you just use regular assertions you'll get an error message that is a little more informative (as it stands, you'll just see "expected true and was

[GitHub] [geode] balesh2 commented on pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

2018-10-11 Thread GitHub
Does removing this not make the test flaky? Seems like it would have the potential to fail on that verify. I have the same question about removing lines 370 and 443 (in the old file's numbering) [ Full content available at: https://github.com/apache/geode/pull/2589 ] This message was relayed

[GitHub] [geode] galen-pivotal commented on pull request #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-11 Thread GitHub
Good cleanup here. However, per [the style guide](https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide), "Always use braces, even around one-line `if`, `else` and other control statements." [ Full content available at: https://github.com/apache/geode/pull/2581 ] This message

[GitHub] [geode] metatype commented on pull request #2591: GEODE-5848 conditionally add --add-opens for test jvm versions 9 and up

2018-10-11 Thread GitHub
Just curious, is this related to adding the JAXB dependencies or does it solve a different issue? [ Full content available at: https://github.com/apache/geode/pull/2591 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] galen-pivotal commented on pull request #2581: GEODE-5830: Fix `NONE` enum for SSL configuration

2018-10-11 Thread GitHub
Suuuper minor grammatical suggestion: put a comma after "all", so that there are clearly three commma-separated options. [ Full content available at: https://github.com/apache/geode/pull/2581 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] sboorlagadda closed pull request #2586: GEODE-5212: fix timing issues in AfterCompletionTest

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

[GitHub] [geode] WireBaron commented on pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

2018-10-11 Thread GitHub
No, the suspect state we're checking is set synchronously in the suspect call above. I'm not sure why this sleep was ever here. InstallView also synchronously sets all the state needed for the test, making the old sleeps at 370 and 443 similarly pointless. [ Full content available at:

[GitHub] [geode] WireBaron closed pull request #2589: GEODE-5807: Refactor flakey GMSHealthMonitorJUnitTest tests by removi…

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

[GitHub] [geode] PurelyApplied commented on pull request #2593: GEODE-5850: Do not build jar in test.

2018-10-11 Thread GitHub
Similar to the other comment, there's no real reason this has to get packaged in the `jar` to the full `o.a.g.m.i.c.c` package. Would this test read cleaner if the jar put the `SerializableNameContainer` at the top-level package? I know I'm overthinking this. [ Full content available at:

[GitHub] [geode] PurelyApplied commented on pull request #2593: GEODE-5850: Do not build jar in test.

2018-10-11 Thread GitHub
Any opinions on where this data container class should live? I kept it on the path to be the same as the test that needs it, but that's not really necessary. It could belong to `geode-dunit` as a shared resource, if we track down other places that need a simple `java.io.Serializable` data

[GitHub] [geode] mhansonp commented on issue #2595: GEODE-5853: fix to check if testJVM is empty

2018-10-11 Thread GitHub
@pivotal-jbarrett @onichols-pivotal Please review this build fix. [ Full content available at: https://github.com/apache/geode/pull/2595 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

  1   2   3   4   5   6   7   8   9   10   >