Review Request 53108: GEODE-2012: always write stat types to archive

[Kirk Lund]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53108/
---

Review request for geode, Anthony Baker, Darrel Schneider, Jinmei Liao, Kevin 
Duling, Swapnil Bawaskar, and Dan Smith.


Bugs: GEODE-2012
https://issues.apache.org/jira/browse/GEODE-2012


Repository: geode


Description
---

GEODE-2012: always write stat types to archive

Note: there are other minor commits on feature/GEODE-2012 that should be 
rebased into one commit before committing to develop.


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/NanoTimer.java 
e8c11456353f0ddb7718cb07da146fa75492995f 
  geode-core/src/main/java/org/apache/geode/internal/i18n/LocalizedStrings.java 
8bfdd686eb50605e12cb59015d5ddab99714c563 
  
geode-core/src/main/java/org/apache/geode/internal/statistics/HostStatSampler.java
 020b5ab8530783f2c76279f8d130f84496013e3a 
  
geode-core/src/main/java/org/apache/geode/internal/statistics/SampleCollector.java
 68430a1d830d9f560fb7131d2c567edc6337b7cb 
  
geode-core/src/main/java/org/apache/geode/internal/statistics/SimpleStatSampler.java
 50f49a70152d45726090d9e3b63ef3daef6008e5 
  
geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveWriter.java
 b8f44e2d50715c5f28132246465cc567c4702946 
  
geode-core/src/main/java/org/apache/geode/internal/util/concurrent/StoppableCountDownLatch.java
 1f0ac34e534e219a55ad73dcdab295f368ba86ae 
  
geode-core/src/test/java/org/apache/geode/internal/statistics/DiskSpaceLimitIntegrationTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/statistics/FileSizeLimitIntegrationTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/statistics/StatTypesAreRolledOverRegressionTest.java
 PRE-CREATION 

Diff: https://reviews.apache.org/r/53108/diff/


Testing
---

precheckin


Thanks,

Kirk Lund

Re: Coding practices/standards

[Jared Stewart]

Try this commit hash instead: d0175ec5aa8acf1b34ece3183fe03e9874450cbb (from 
feature/spotlessPlugin).


> On Oct 21, 2016, at 4:16 PM, Kirk Lund  wrote:
> 
> FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the Apache
> git repo.
> 
> Is there a way to reformat a branch and then rebase on develop to minimize
> conflicts?
> 
> -Kirk
> 
> 
> On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart  wrote:
> 
>> Fantastic, thanks for merging this in Mark.  For anyone with outstanding
>> work on branches made before this change, your life may be made easier by
>> cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
>> Spotless) into your branch and then running ‘gradlew spotlessApply’ on it
>> before attempting to merge into develop.
>> 
>> — Jared
>>> On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:
>>> 
>>> Thanks Jared for the suggestion of Spotless and follow-up work.
>>> 
>>> This is now completed and checked into develop. As this does touch many
>>> files, be prepared the next time you pull.
>>> 
>>> --Mark
>>> 
>>> 
>>> 
>>> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
>> wrote:
>>> 
 Done! :)
 
 - Jared
> On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
> 
> One more time! :)
> 
> Conflicting files
> geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXDUnitTest.java
> geode-core/src/test/java/org/apache/geode/disttx/
 PRDistTXWithVersionsDUnitTest.java
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/
 PRTransactionDUnitTest.java
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
 wrote:
> 
>> I just pulled and rebased onto develop, and force pushed into the
 existing
>> pull request.  It should be clean to merge in now.
>> 
>> Thanks,
>> Jared
>>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
>> wrote:
>>> 
>>> I believe there is enough consensus here to check this into develop.
>>> 
>>> Jared, due to recent checkins into develop, can you update the pull
>> request
>>> one more time? Trying to make this as clean as possible. I will check
>> into
>>> develop after the update, unless someone else gets to it first.
>>> 
>>> All, can we hold checkins on develop until the new formatter is
 applied?
>>> 
>>> Thanks,
>>> 
>>> --Mark
>>> 
>>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
 wrote:
>>> 
 +1
 
> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
 bschucha...@pivotal.io>
 wrote:
> 
> +1
> 
> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
>> +1
>> 
>> 
>> On 20/10/16 4:56 pm, Mark Bretl wrote:
>>> +1 as well...
>>> 
>>> - Pulled changes
>>> - Executed './gradlew clean build' and was successful.
>>> - Modified a couple of random files to test
>>> - Ran './gradlew clean build' again and failed expectedly
>>> - Ran './gradlew spotlessApply', task was successful
>>> - Ran './gradlew clean build' and succeeded
>>> 
>>> Great addition! As long as others are good with the formatter,
 then I
 am
>>> good.
>>> 
>>> --Mark
>>> 
>>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
 wrote:
>>> 
 +1 I just added my approval to the PR (and again here)
 
 
 On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
 jstew...@pivotal.io
>>> 
 wrote:
 
> I have opened a pull request here  incubator-geode/pull/268> to enable the Spotless plugin and to
 switch to
> the Google Java Style formatter templates.
> 
> 
>> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
 wrote:
>> 
>> For reference TRAC #38741 was a bug with the summary
 "EOFException
 during
>> deserialize on client update with copy-on-read=true"
>> 
>> -Kirk
>> 
>> 
>> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
>> jstew...@pivotal.io
> 
> wrote:
>>> To give everyone an update, using the Google Java Style
>> eclipse
 template
>>> there is an issue spotlessCheck where fails for
>>> geode-core/src/test/java/org/apache/geode/cache30/
> Bug38741DUnitTest.java
>>> even if you run it directly after spotlessApply. This needs
>> to
 be
>>> investigated and fixed before I can open a pull request to

Re: Review Request 53094: GEODE-706 race condition between expiry thread and other user thread.

[Bruce Schuchardt]

That's a better comment.  You should replace the one you have with it.

Le 10/21/2016 à 4:03 PM, Hitesh Khamesra a écrit :
This is an automatically generated e-mail. To reply, visit: 
https://reviews.apache.org/r/53094/



On October 21st, 2016, 10:36 p.m. UTC, *Bruce Schuchardt* wrote:


geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java


(Diff revision 1)

public final boolean destroy(EntryEventImpl event,

1516

 owner.cancelExpiryTask(re,  event.getExpiryTask());

1516

 //we only want to cancel if concurrency-check is not 
enabled

I don't understand this comment. There doesn't seem to be a
check for concurrency-checks being enabled & you've removed
the check for a tombstone.

Right, I just added some more comment there. Basically regionEntry 
will be null if concurrency-check is enable. And in that case we 
cancel expiry from removeTomstone method.



//we only want to cancel if concurrency-check is not enabled 
//re(regionentry) will be null when concurrency-check is enable and 
removeTombstone method //will call cancelExpiryTask on regionEntry



- Hitesh


On October 21st, 2016, 6:41 p.m. UTC, Hitesh Khamesra wrote:

Review request for geode, Bruce Schuchardt, Darrel Schneider, and Udo 
Kohlmeyer.

By Hitesh Khamesra.

/Updated Oct. 21, 2016, 6:41 p.m./

*Repository: * geode


  Description

ExpirationManager tracks the regionEntry as reference to expiryTask. 
It assumes, it is unique for that key. But consistency check mechanism 
keeps that regionEntry around and that enforces re-use of that for new 
update. That introduces the race condition between expiry thread and 
user thread, it endup not scheduling the entry for expiration. As a 
fix, now "consistency check mechanism" unschedules the expiry task to 
avoid this race condition.



  Diffs

  * 
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
(e02c7e1)
  * geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java
(d059aab)
  * 
geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
(0c20d32)
  * geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
(ac4c705)

View Diff 

Re: Coding practices/standards

[Kirk Lund]

FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the Apache
git repo.

Is there a way to reformat a branch and then rebase on develop to minimize
conflicts?

-Kirk


On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart  wrote:

> Fantastic, thanks for merging this in Mark.  For anyone with outstanding
> work on branches made before this change, your life may be made easier by
> cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
> Spotless) into your branch and then running ‘gradlew spotlessApply’ on it
> before attempting to merge into develop.
>
> — Jared
> > On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:
> >
> > Thanks Jared for the suggestion of Spotless and follow-up work.
> >
> > This is now completed and checked into develop. As this does touch many
> > files, be prepared the next time you pull.
> >
> > --Mark
> >
> >
> >
> > On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> wrote:
> >
> >> Done! :)
> >>
> >> - Jared
> >>> On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
> >>>
> >>> One more time! :)
> >>>
> >>> Conflicting files
> >>> geode-core/src/test/java/org/apache/geode/disttx/
> PRDistTXDUnitTest.java
> >>> geode-core/src/test/java/org/apache/geode/disttx/
> >> PRDistTXWithVersionsDUnitTest.java
> >>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> >> PRTransactionDUnitTest.java
> >>>
> >>> --Mark
> >>>
> >>> On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
> >> wrote:
> >>>
>  I just pulled and rebased onto develop, and force pushed into the
> >> existing
>  pull request.  It should be clean to merge in now.
> 
>  Thanks,
>  Jared
> > On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> wrote:
> >
> > I believe there is enough consensus here to check this into develop.
> >
> > Jared, due to recent checkins into develop, can you update the pull
>  request
> > one more time? Trying to make this as clean as possible. I will check
>  into
> > develop after the update, unless someone else gets to it first.
> >
> > All, can we hold checkins on develop until the new formatter is
> >> applied?
> >
> > Thanks,
> >
> > --Mark
> >
> > On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> >> wrote:
> >
> >> +1
> >>
> >>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> >> bschucha...@pivotal.io>
> >> wrote:
> >>>
> >>> +1
> >>>
> >>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
>  +1
> 
> 
>  On 20/10/16 4:56 pm, Mark Bretl wrote:
> > +1 as well...
> >
> > - Pulled changes
> > - Executed './gradlew clean build' and was successful.
> > - Modified a couple of random files to test
> > - Ran './gradlew clean build' again and failed expectedly
> > - Ran './gradlew spotlessApply', task was successful
> > - Ran './gradlew clean build' and succeeded
> >
> > Great addition! As long as others are good with the formatter,
> >> then I
> >> am
> > good.
> >
> > --Mark
> >
> > On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> >> wrote:
> >
> >> +1 I just added my approval to the PR (and again here)
> >>
> >>
> >> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> >> jstew...@pivotal.io
> >
> >> wrote:
> >>
> >>> I have opened a pull request here  >>> incubator-geode/pull/268> to enable the Spotless plugin and to
> >> switch to
> >>> the Google Java Style formatter templates.
> >>>
> >>>
>  On Oct 18, 2016, at 4:32 PM, Kirk Lund 
> >> wrote:
> 
>  For reference TRAC #38741 was a bug with the summary
> >> "EOFException
> >> during
>  deserialize on client update with copy-on-read=true"
> 
>  -Kirk
> 
> 
>  On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
>  jstew...@pivotal.io
> >>>
> >>> wrote:
> > To give everyone an update, using the Google Java Style
> eclipse
> >> template
> > there is an issue spotlessCheck where fails for
> > geode-core/src/test/java/org/apache/geode/cache30/
> >>> Bug38741DUnitTest.java
> > even if you run it directly after spotlessApply. This needs
> to
> >> be
> > investigated and fixed before I can open a pull request to
> >> enable
> >>> spotless.
> >
> >> On Oct 14, 2016, at 4:57 PM, Dan Smith 
>  wrote:
> >>
> >> +1 - The formatting looks better now.
> >>
> >> -Dan

Re: Review Request 53092: GEODE-2011 Client clears pdx registry needs synchronization

[Hitesh Khamesra]

> On Oct. 21, 2016, 10:25 p.m., Bruce Schuchardt wrote:
> > is it possible to unit test this change?  
> > PdxClientServerDUnitTest.testNonPersistentServerRestartAutoSerializer ran 
> > into the problem but seems to be marked Flaky.  Can you modify that test to 
> > always hit the problem and remove the Flaky label?

Possibly we can do that by introducing some testHook and call from 
PdxRegistoryRecoveryListener and then just wait "before clearing the pdx 
Registry".


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53092/#review153605
---


On Oct. 21, 2016, 6:52 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53092/
> ---
> 
> (Updated Oct. 21, 2016, 6:52 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Udo Kohlmeyer, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> We clear pdx registry in client when we re-connect to cluster. But during 
> that time other thread may end up using old registry while other thread is 
> clearing this. Thus we need to synchronize that. In current code it happens 
> through listener events and I modified that notification to synchronize this.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java
>  3f3d725 
> 
> Diff: https://reviews.apache.org/r/53092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 53094: GEODE-706 race condition between expiry thread and other user thread.

[Hitesh Khamesra]

> On Oct. 21, 2016, 10:36 p.m., Bruce Schuchardt wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java,
> >  line 1516
> > 
> >
> > I don't understand this comment.  There doesn't seem to be a check for 
> > concurrency-checks being enabled & you've removed the check for a tombstone.

Right, I just added some more comment there. Basically regionEntry will be null 
if concurrency-check is enable. And in that case we cancel expiry from 
removeTomstone method.

//we only want to cancel if concurrency-check is not enabled
//re(regionentry) will be null when concurrency-check is enable and 
removeTombstone method
//will call cancelExpiryTask on regionEntry


- Hitesh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53094/#review153607
---


On Oct. 21, 2016, 6:41 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53094/
> ---
> 
> (Updated Oct. 21, 2016, 6:41 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> ExpirationManager tracks the regionEntry as reference to expiryTask. It 
> assumes, it is unique for that key. But consistency check mechanism keeps 
> that regionEntry around and that enforces re-use of that for new update. That 
> introduces the race condition between expiry thread and user thread, it endup 
> not scheduling the entry for expiration. As a fix, now "consistency check 
> mechanism" unschedules the expiry task to avoid this race condition.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  e02c7e1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java 
> d059aab 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java 
> 0c20d32 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> ac4c705 
> 
> Diff: https://reviews.apache.org/r/53094/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Review Request 53107: GEODE-2023: Add Lucene documentation

[Dave Barnes]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53107/
---

Review request for geode, Joey McAllister and Karen Miller.


Repository: geode


Description
---

GEODE-2023: Add Lucene documentation


Diffs
-

  geode-book/master_middleman/source/subnavs/geode-subnav.erb 
2373f4b397952372a61923c5b57ef8f6d2102ee3 
  geode-docs/tools_modules/book_intro.html.md.erb 
852e3c9de6eba2f83f0c513912c53b771889509e 
  geode-docs/tools_modules/lucene_integration.html.md.erb PRE-CREATION 

Diff: https://reviews.apache.org/r/53107/diff/


Testing
---


Thanks,

Dave Barnes

Re: Review Request 53094: GEODE-706 race condition between expiry thread and other user thread.

[Bruce Schuchardt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53094/#review153607
---




geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java 
(line 1516)


I don't understand this comment.  There doesn't seem to be a check for 
concurrency-checks being enabled & you've removed the check for a tombstone.


- Bruce Schuchardt


On Oct. 21, 2016, 6:41 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53094/
> ---
> 
> (Updated Oct. 21, 2016, 6:41 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> ExpirationManager tracks the regionEntry as reference to expiryTask. It 
> assumes, it is unique for that key. But consistency check mechanism keeps 
> that regionEntry around and that enforces re-use of that for new update. That 
> introduces the race condition between expiry thread and user thread, it endup 
> not scheduling the entry for expiration. As a fix, now "consistency check 
> mechanism" unschedules the expiry task to avoid this race condition.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  e02c7e1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java 
> d059aab 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java 
> 0c20d32 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> ac4c705 
> 
> Diff: https://reviews.apache.org/r/53094/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Review Request 53092: GEODE-2011 Client clears pdx registry needs synchronization

[Bruce Schuchardt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53092/#review153605
---



is it possible to unit test this change?  
PdxClientServerDUnitTest.testNonPersistentServerRestartAutoSerializer ran into 
the problem but seems to be marked Flaky.  Can you modify that test to always 
hit the problem and remove the Flaky label?

- Bruce Schuchardt


On Oct. 21, 2016, 6:52 p.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53092/
> ---
> 
> (Updated Oct. 21, 2016, 6:52 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Udo Kohlmeyer, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> We clear pdx registry in client when we re-connect to cluster. But during 
> that time other thread may end up using old registry while other thread is 
> clearing this. Thus we need to synchronize that. In current code it happens 
> through listener events and I modified that notification to synchronize this.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java
>  3f3d725 
> 
> Diff: https://reviews.apache.org/r/53092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Coding practices/standards

[William Markito]

This looks pretty good indeed! Thanks Jared!

On Fri, Oct 21, 2016 at 3:10 PM, Dan Smith  wrote:

> We've seen the sync to github lag before when there are lots of changes,
> like when the docs were contributed. Hopefully the changes should show up
> there shortly!
>
> -Dan
>
> On Fri, Oct 21, 2016 at 2:53 PM, Jared Stewart 
> wrote:
>
> > By the way, when I pull develop from GitHub I don’t see these changes.
> > Nor do I see them when I look here  > incubator-geode/commits/develop>.  It seems that they were added to the
> > apache git repo but not the apache GitHub repo,  maybe due to the GitHub
> > DNS issues going on today?  Anyways just wanted to see if anyone else
> has a
> > better explanation.
> >
> > —Jared
> > > On Oct 21, 2016, at 1:38 PM, Kirk Lund  wrote:
> > >
> > > I just did a pull and now I'm reviewing the formatters under etc/.
> > >
> > > -rw-rw-r--  1 klund  staff  36004 Oct 21 13:36
> > eclipse-java-google-style.xml
> > > -rw-rw-r--  1 klund  staff110 Sep 19 11:56
> > > eclipseOrganizeImports.importorder
> > > -rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
> > > intellij-java-google-style.xml
> > >
> > > What's the status of organizing imports? And do we still use
> > > eclipseOrganizeImports.importorder for imports in Eclipse?
> > >
> > > Thanks,
> > > Kirk
> > >
> > >
> > > On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl 
> > wrote:
> > >
> > >> Thanks Jared for the suggestion of Spotless and follow-up work.
> > >>
> > >> This is now completed and checked into develop. As this does touch
> many
> > >> files, be prepared the next time you pull.
> > >>
> > >> --Mark
> > >>
> > >>
> > >>
> > >> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> > >> wrote:
> > >>
> > >>> Done! :)
> > >>>
> > >>> - Jared
> >  On Oct 21, 2016, at 12:27 PM, Mark Bretl 
> > wrote:
> > 
> >  One more time! :)
> > 
> >  Conflicting files
> >  geode-core/src/test/java/org/apache/geode/disttx/
> > >> PRDistTXDUnitTest.java
> >  geode-core/src/test/java/org/apache/geode/disttx/
> > >>> PRDistTXWithVersionsDUnitTest.java
> >  geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> > >>> PRTransactionDUnitTest.java
> > 
> >  --Mark
> > 
> >  On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart <
> jstew...@pivotal.io>
> > >>> wrote:
> > 
> > > I just pulled and rebased onto develop, and force pushed into the
> > >>> existing
> > > pull request.  It should be clean to merge in now.
> > >
> > > Thanks,
> > > Jared
> > >> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> > >> wrote:
> > >>
> > >> I believe there is enough consensus here to check this into
> develop.
> > >>
> > >> Jared, due to recent checkins into develop, can you update the
> pull
> > > request
> > >> one more time? Trying to make this as clean as possible. I will
> > check
> > > into
> > >> develop after the update, unless someone else gets to it first.
> > >>
> > >> All, can we hold checkins on develop until the new formatter is
> > >>> applied?
> > >>
> > >> Thanks,
> > >>
> > >> --Mark
> > >>
> > >> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> > >>> wrote:
> > >>
> > >>> +1
> > >>>
> >  On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> > >>> bschucha...@pivotal.io>
> > >>> wrote:
> > 
> >  +1
> > 
> >  Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> > > +1
> > >
> > >
> > > On 20/10/16 4:56 pm, Mark Bretl wrote:
> > >> +1 as well...
> > >>
> > >> - Pulled changes
> > >> - Executed './gradlew clean build' and was successful.
> > >> - Modified a couple of random files to test
> > >> - Ran './gradlew clean build' again and failed expectedly
> > >> - Ran './gradlew spotlessApply', task was successful
> > >> - Ran './gradlew clean build' and succeeded
> > >>
> > >> Great addition! As long as others are good with the formatter,
> > >>> then I
> > >>> am
> > >> good.
> > >>
> > >> --Mark
> > >>
> > >> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> > >>> wrote:
> > >>
> > >>> +1 I just added my approval to the PR (and again here)
> > >>>
> > >>>
> > >>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> > >>> jstew...@pivotal.io
> > >>
> > >>> wrote:
> > >>>
> >  I have opened a pull request here <
> https://github.com/apache/
> >  incubator-geode/pull/268> to enable the Spotless plugin and
> to
> > >>> switch to
> >  the Google Java Style formatter templates.
> > 
> > 
> > 

Re: Coding practices/standards

[Dan Smith]

We've seen the sync to github lag before when there are lots of changes,
like when the docs were contributed. Hopefully the changes should show up
there shortly!

-Dan

On Fri, Oct 21, 2016 at 2:53 PM, Jared Stewart  wrote:

> By the way, when I pull develop from GitHub I don’t see these changes.
> Nor do I see them when I look here  incubator-geode/commits/develop>.  It seems that they were added to the
> apache git repo but not the apache GitHub repo,  maybe due to the GitHub
> DNS issues going on today?  Anyways just wanted to see if anyone else has a
> better explanation.
>
> —Jared
> > On Oct 21, 2016, at 1:38 PM, Kirk Lund  wrote:
> >
> > I just did a pull and now I'm reviewing the formatters under etc/.
> >
> > -rw-rw-r--  1 klund  staff  36004 Oct 21 13:36
> eclipse-java-google-style.xml
> > -rw-rw-r--  1 klund  staff110 Sep 19 11:56
> > eclipseOrganizeImports.importorder
> > -rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
> > intellij-java-google-style.xml
> >
> > What's the status of organizing imports? And do we still use
> > eclipseOrganizeImports.importorder for imports in Eclipse?
> >
> > Thanks,
> > Kirk
> >
> >
> > On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl 
> wrote:
> >
> >> Thanks Jared for the suggestion of Spotless and follow-up work.
> >>
> >> This is now completed and checked into develop. As this does touch many
> >> files, be prepared the next time you pull.
> >>
> >> --Mark
> >>
> >>
> >>
> >> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> >> wrote:
> >>
> >>> Done! :)
> >>>
> >>> - Jared
>  On Oct 21, 2016, at 12:27 PM, Mark Bretl 
> wrote:
> 
>  One more time! :)
> 
>  Conflicting files
>  geode-core/src/test/java/org/apache/geode/disttx/
> >> PRDistTXDUnitTest.java
>  geode-core/src/test/java/org/apache/geode/disttx/
> >>> PRDistTXWithVersionsDUnitTest.java
>  geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> >>> PRTransactionDUnitTest.java
> 
>  --Mark
> 
>  On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
> >>> wrote:
> 
> > I just pulled and rebased onto develop, and force pushed into the
> >>> existing
> > pull request.  It should be clean to merge in now.
> >
> > Thanks,
> > Jared
> >> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> >> wrote:
> >>
> >> I believe there is enough consensus here to check this into develop.
> >>
> >> Jared, due to recent checkins into develop, can you update the pull
> > request
> >> one more time? Trying to make this as clean as possible. I will
> check
> > into
> >> develop after the update, unless someone else gets to it first.
> >>
> >> All, can we hold checkins on develop until the new formatter is
> >>> applied?
> >>
> >> Thanks,
> >>
> >> --Mark
> >>
> >> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> >>> wrote:
> >>
> >>> +1
> >>>
>  On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> >>> bschucha...@pivotal.io>
> >>> wrote:
> 
>  +1
> 
>  Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> > +1
> >
> >
> > On 20/10/16 4:56 pm, Mark Bretl wrote:
> >> +1 as well...
> >>
> >> - Pulled changes
> >> - Executed './gradlew clean build' and was successful.
> >> - Modified a couple of random files to test
> >> - Ran './gradlew clean build' again and failed expectedly
> >> - Ran './gradlew spotlessApply', task was successful
> >> - Ran './gradlew clean build' and succeeded
> >>
> >> Great addition! As long as others are good with the formatter,
> >>> then I
> >>> am
> >> good.
> >>
> >> --Mark
> >>
> >> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> >>> wrote:
> >>
> >>> +1 I just added my approval to the PR (and again here)
> >>>
> >>>
> >>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> >>> jstew...@pivotal.io
> >>
> >>> wrote:
> >>>
>  I have opened a pull request here   incubator-geode/pull/268> to enable the Spotless plugin and to
> >>> switch to
>  the Google Java Style formatter templates.
> 
> 
> > On Oct 18, 2016, at 4:32 PM, Kirk Lund 
> >>> wrote:
> >
> > For reference TRAC #38741 was a bug with the summary
> >>> "EOFException
> >>> during
> > deserialize on client update with copy-on-read=true"
> >
> > -Kirk
> >
> >
> > On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> > 

Re: Coding practices/standards

[Jared Stewart]

By the way, when I pull develop from GitHub I don’t see these changes.  Nor do 
I see them when I look here 
.  It seems that 
they were added to the apache git repo but not the apache GitHub repo,  maybe 
due to the GitHub DNS issues going on today?  Anyways just wanted to see if 
anyone else has a better explanation.

—Jared
> On Oct 21, 2016, at 1:38 PM, Kirk Lund  wrote:
> 
> I just did a pull and now I'm reviewing the formatters under etc/.
> 
> -rw-rw-r--  1 klund  staff  36004 Oct 21 13:36 eclipse-java-google-style.xml
> -rw-rw-r--  1 klund  staff110 Sep 19 11:56
> eclipseOrganizeImports.importorder
> -rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
> intellij-java-google-style.xml
> 
> What's the status of organizing imports? And do we still use
> eclipseOrganizeImports.importorder for imports in Eclipse?
> 
> Thanks,
> Kirk
> 
> 
> On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl  wrote:
> 
>> Thanks Jared for the suggestion of Spotless and follow-up work.
>> 
>> This is now completed and checked into develop. As this does touch many
>> files, be prepared the next time you pull.
>> 
>> --Mark
>> 
>> 
>> 
>> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
>> wrote:
>> 
>>> Done! :)
>>> 
>>> - Jared
 On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
 
 One more time! :)
 
 Conflicting files
 geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXDUnitTest.java
 geode-core/src/test/java/org/apache/geode/disttx/
>>> PRDistTXWithVersionsDUnitTest.java
 geode-core/src/test/java/org/apache/geode/internal/cache/execute/
>>> PRTransactionDUnitTest.java
 
 --Mark
 
 On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
>>> wrote:
 
> I just pulled and rebased onto develop, and force pushed into the
>>> existing
> pull request.  It should be clean to merge in now.
> 
> Thanks,
> Jared
>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
>> wrote:
>> 
>> I believe there is enough consensus here to check this into develop.
>> 
>> Jared, due to recent checkins into develop, can you update the pull
> request
>> one more time? Trying to make this as clean as possible. I will check
> into
>> develop after the update, unless someone else gets to it first.
>> 
>> All, can we hold checkins on develop until the new formatter is
>>> applied?
>> 
>> Thanks,
>> 
>> --Mark
>> 
>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
>>> wrote:
>> 
>>> +1
>>> 
 On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
>>> bschucha...@pivotal.io>
>>> wrote:
 
 +1
 
 Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> +1
> 
> 
> On 20/10/16 4:56 pm, Mark Bretl wrote:
>> +1 as well...
>> 
>> - Pulled changes
>> - Executed './gradlew clean build' and was successful.
>> - Modified a couple of random files to test
>> - Ran './gradlew clean build' again and failed expectedly
>> - Ran './gradlew spotlessApply', task was successful
>> - Ran './gradlew clean build' and succeeded
>> 
>> Great addition! As long as others are good with the formatter,
>>> then I
>>> am
>> good.
>> 
>> --Mark
>> 
>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
>>> wrote:
>> 
>>> +1 I just added my approval to the PR (and again here)
>>> 
>>> 
>>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
>>> jstew...@pivotal.io
>> 
>>> wrote:
>>> 
 I have opened a pull request here  to enable the Spotless plugin and to
>>> switch to
 the Google Java Style formatter templates.
 
 
> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
>>> wrote:
> 
> For reference TRAC #38741 was a bug with the summary
>>> "EOFException
>>> during
> deserialize on client update with copy-on-read=true"
> 
> -Kirk
> 
> 
> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> jstew...@pivotal.io
 
 wrote:
>> To give everyone an update, using the Google Java Style
>> eclipse
>>> template
>> there is an issue spotlessCheck where fails for
>> geode-core/src/test/java/org/apache/geode/cache30/
 Bug38741DUnitTest.java
>> even if you run it directly after spotlessApply. This needs
>> to
>>> be
>> investigated and fixed before I can open a pull request to
>>> enable

Re: Review Request 53094: GEODE-706 race condition between expiry thread and other user thread.

[Darrel Schneider]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53094/#review153596
---




geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java (line 
8660)


It would be better to remove these calls instead of just commenting them 
out.


- Darrel Schneider


On Oct. 21, 2016, 11:41 a.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53094/
> ---
> 
> (Updated Oct. 21, 2016, 11:41 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Darrel Schneider, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> ExpirationManager tracks the regionEntry as reference to expiryTask. It 
> assumes, it is unique for that key. But consistency check mechanism keeps 
> that regionEntry around and that enforces re-use of that for new update. That 
> introduces the race condition between expiry thread and user thread, it endup 
> not scheduling the entry for expiration. As a fix, now "consistency check 
> mechanism" unschedules the expiry task to avoid this race condition.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
>  e02c7e1 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java 
> d059aab 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java 
> 0c20d32 
>   geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
> ac4c705 
> 
> Diff: https://reviews.apache.org/r/53094/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Re: Coding practices/standards

[Jared Stewart]

I haven’t dug in to the configuration of eclipseOrganizeImports, so for now 
spotless is not enforcing import ordering.  If we verify that the import 
ordering specified by this file looks good, I can turn that on in the spotless 
config.

— Jared
 
> On Oct 21, 2016, at 1:38 PM, Kirk Lund  wrote:
> 
> I just did a pull and now I'm reviewing the formatters under etc/.
> 
> -rw-rw-r--  1 klund  staff  36004 Oct 21 13:36 eclipse-java-google-style.xml
> -rw-rw-r--  1 klund  staff110 Sep 19 11:56
> eclipseOrganizeImports.importorder
> -rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
> intellij-java-google-style.xml
> 
> What's the status of organizing imports? And do we still use
> eclipseOrganizeImports.importorder for imports in Eclipse?
> 
> Thanks,
> Kirk
> 
> 
> On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl  wrote:
> 
>> Thanks Jared for the suggestion of Spotless and follow-up work.
>> 
>> This is now completed and checked into develop. As this does touch many
>> files, be prepared the next time you pull.
>> 
>> --Mark
>> 
>> 
>> 
>> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
>> wrote:
>> 
>>> Done! :)
>>> 
>>> - Jared
 On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
 
 One more time! :)
 
 Conflicting files
 geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXDUnitTest.java
 geode-core/src/test/java/org/apache/geode/disttx/
>>> PRDistTXWithVersionsDUnitTest.java
 geode-core/src/test/java/org/apache/geode/internal/cache/execute/
>>> PRTransactionDUnitTest.java
 
 --Mark
 
 On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
>>> wrote:
 
> I just pulled and rebased onto develop, and force pushed into the
>>> existing
> pull request.  It should be clean to merge in now.
> 
> Thanks,
> Jared
>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
>> wrote:
>> 
>> I believe there is enough consensus here to check this into develop.
>> 
>> Jared, due to recent checkins into develop, can you update the pull
> request
>> one more time? Trying to make this as clean as possible. I will check
> into
>> develop after the update, unless someone else gets to it first.
>> 
>> All, can we hold checkins on develop until the new formatter is
>>> applied?
>> 
>> Thanks,
>> 
>> --Mark
>> 
>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
>>> wrote:
>> 
>>> +1
>>> 
 On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
>>> bschucha...@pivotal.io>
>>> wrote:
 
 +1
 
 Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> +1
> 
> 
> On 20/10/16 4:56 pm, Mark Bretl wrote:
>> +1 as well...
>> 
>> - Pulled changes
>> - Executed './gradlew clean build' and was successful.
>> - Modified a couple of random files to test
>> - Ran './gradlew clean build' again and failed expectedly
>> - Ran './gradlew spotlessApply', task was successful
>> - Ran './gradlew clean build' and succeeded
>> 
>> Great addition! As long as others are good with the formatter,
>>> then I
>>> am
>> good.
>> 
>> --Mark
>> 
>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
>>> wrote:
>> 
>>> +1 I just added my approval to the PR (and again here)
>>> 
>>> 
>>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
>>> jstew...@pivotal.io
>> 
>>> wrote:
>>> 
 I have opened a pull request here  to enable the Spotless plugin and to
>>> switch to
 the Google Java Style formatter templates.
 
 
> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
>>> wrote:
> 
> For reference TRAC #38741 was a bug with the summary
>>> "EOFException
>>> during
> deserialize on client update with copy-on-read=true"
> 
> -Kirk
> 
> 
> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> jstew...@pivotal.io
 
 wrote:
>> To give everyone an update, using the Google Java Style
>> eclipse
>>> template
>> there is an issue spotlessCheck where fails for
>> geode-core/src/test/java/org/apache/geode/cache30/
 Bug38741DUnitTest.java
>> even if you run it directly after spotlessApply. This needs
>> to
>>> be
>> investigated and fixed before I can open a pull request to
>>> enable
 spotless.
>> 
>>> On Oct 14, 2016, at 4:57 PM, Dan Smith 
> wrote:

Re: Coding practices/standards

[Kirk Lund]

I just did a pull and now I'm reviewing the formatters under etc/.

-rw-rw-r--  1 klund  staff  36004 Oct 21 13:36 eclipse-java-google-style.xml
-rw-rw-r--  1 klund  staff110 Sep 19 11:56
eclipseOrganizeImports.importorder
-rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
intellij-java-google-style.xml

What's the status of organizing imports? And do we still use
eclipseOrganizeImports.importorder for imports in Eclipse?

Thanks,
Kirk


On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl  wrote:

> Thanks Jared for the suggestion of Spotless and follow-up work.
>
> This is now completed and checked into develop. As this does touch many
> files, be prepared the next time you pull.
>
> --Mark
>
>
>
> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> wrote:
>
> > Done! :)
> >
> > - Jared
> > > On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
> > >
> > > One more time! :)
> > >
> > > Conflicting files
> > > geode-core/src/test/java/org/apache/geode/disttx/
> PRDistTXDUnitTest.java
> > > geode-core/src/test/java/org/apache/geode/disttx/
> > PRDistTXWithVersionsDUnitTest.java
> > > geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> > PRTransactionDUnitTest.java
> > >
> > > --Mark
> > >
> > > On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
> > wrote:
> > >
> > >> I just pulled and rebased onto develop, and force pushed into the
> > existing
> > >> pull request.  It should be clean to merge in now.
> > >>
> > >> Thanks,
> > >> Jared
> > >>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> wrote:
> > >>>
> > >>> I believe there is enough consensus here to check this into develop.
> > >>>
> > >>> Jared, due to recent checkins into develop, can you update the pull
> > >> request
> > >>> one more time? Trying to make this as clean as possible. I will check
> > >> into
> > >>> develop after the update, unless someone else gets to it first.
> > >>>
> > >>> All, can we hold checkins on develop until the new formatter is
> > applied?
> > >>>
> > >>> Thanks,
> > >>>
> > >>> --Mark
> > >>>
> > >>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> > wrote:
> > >>>
> >  +1
> > 
> > > On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> > bschucha...@pivotal.io>
> >  wrote:
> > >
> > > +1
> > >
> > > Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> > >> +1
> > >>
> > >>
> > >> On 20/10/16 4:56 pm, Mark Bretl wrote:
> > >>> +1 as well...
> > >>>
> > >>> - Pulled changes
> > >>> - Executed './gradlew clean build' and was successful.
> > >>> - Modified a couple of random files to test
> > >>> - Ran './gradlew clean build' again and failed expectedly
> > >>> - Ran './gradlew spotlessApply', task was successful
> > >>> - Ran './gradlew clean build' and succeeded
> > >>>
> > >>> Great addition! As long as others are good with the formatter,
> > then I
> >  am
> > >>> good.
> > >>>
> > >>> --Mark
> > >>>
> > >>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> > wrote:
> > >>>
> >  +1 I just added my approval to the PR (and again here)
> > 
> > 
> >  On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> > jstew...@pivotal.io
> > >>>
> >  wrote:
> > 
> > > I have opened a pull request here  > > incubator-geode/pull/268> to enable the Spotless plugin and to
> >  switch to
> > > the Google Java Style formatter templates.
> > >
> > >
> > >> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
> > wrote:
> > >>
> > >> For reference TRAC #38741 was a bug with the summary
> > "EOFException
> >  during
> > >> deserialize on client update with copy-on-read=true"
> > >>
> > >> -Kirk
> > >>
> > >>
> > >> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> > >> jstew...@pivotal.io
> > >
> > > wrote:
> > >>> To give everyone an update, using the Google Java Style
> eclipse
> >  template
> > >>> there is an issue spotlessCheck where fails for
> > >>> geode-core/src/test/java/org/apache/geode/cache30/
> > > Bug38741DUnitTest.java
> > >>> even if you run it directly after spotlessApply. This needs
> to
> > be
> > >>> investigated and fixed before I can open a pull request to
> > enable
> > > spotless.
> > >>>
> >  On Oct 14, 2016, at 4:57 PM, Dan Smith 
> > >> wrote:
> > 
> >  +1 - The formatting looks better now.
> > 
> >  -Dan
> > 
> >  On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
> >  jstew...@pivotal.io
> > >>> wrote:
> > > I agree that the formatter needs fixing up.  Our wiki <

Re: Coding practices/standards

[Udo Kohlmeyer]

+1


On 21/10/16 1:36 pm, Jared Stewart wrote:

Fantastic, thanks for merging this in Mark.  For anyone with outstanding work 
on branches made before this change, your life may be made easier by 
cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added Spotless) 
into your branch and then running ‘gradlew spotlessApply’ on it before 
attempting to merge into develop.

— Jared

On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:

Thanks Jared for the suggestion of Spotless and follow-up work.

This is now completed and checked into develop. As this does touch many
files, be prepared the next time you pull.

--Mark



On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart  wrote:


Done! :)

- Jared

On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:

One more time! :)

Conflicting files
geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java
geode-core/src/test/java/org/apache/geode/disttx/

PRDistTXWithVersionsDUnitTest.java

geode-core/src/test/java/org/apache/geode/internal/cache/execute/

PRTransactionDUnitTest.java

--Mark

On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 

wrote:

I just pulled and rebased onto develop, and force pushed into the

existing

pull request.  It should be clean to merge in now.

Thanks,
Jared

On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:

I believe there is enough consensus here to check this into develop.

Jared, due to recent checkins into develop, can you update the pull

request

one more time? Trying to make this as clean as possible. I will check

into

develop after the update, unless someone else gets to it first.

All, can we hold checkins on develop until the new formatter is

applied?

Thanks,

--Mark

On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 

wrote:

+1


On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <

bschucha...@pivotal.io>

wrote:

+1

Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :

+1


On 20/10/16 4:56 pm, Mark Bretl wrote:

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the formatter,

then I

am

good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 

wrote:

+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <

jstew...@pivotal.io

wrote:


I have opened a pull request here  to enable the Spotless plugin and to

switch to

the Google Java Style formatter templates.



On Oct 18, 2016, at 4:32 PM, Kirk Lund 

wrote:

For reference TRAC #38741 was a bug with the summary

"EOFException

during

deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <

jstew...@pivotal.io

wrote:

To give everyone an update, using the Google Java Style eclipse

template

there is an issue spotlessCheck where fails for
geode-core/src/test/java/org/apache/geode/cache30/

Bug38741DUnitTest.java

even if you run it directly after spotlessApply. This needs to

be

investigated and fixed before I can open a pull request to

enable

spotless.

On Oct 14, 2016, at 4:57 PM, Dan Smith 

wrote:

+1 - The formatting looks better now.

-Dan

On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <

jstew...@pivotal.io

wrote:

I agree that the formatter needs fixing up.  Our wiki <
https://cwiki.apache.org/confluence/display/GEODE/Code+

Style+Guide>

says

that we follow the Google Java Style guide, but that is not

actually

what’s

in our formatter templates.  I pushed a new branch <

https://github.com/

jaredjstewart/incubator-geode/tree/

spotlessPluginGoogleStyle>

that

points

spotless at the actual Google Java Style template, and I

think

it

makes

both of the examples you found look better.


On Oct 13, 2016, at 10:11 AM, Dan Smith 

wrote:

+1 for adding this to ./gradlew build

But I think we might want to fix up the formatter a bit

before

reformatting

the code. I tried running spotlessApply, and it did some

unfortunate

reformatting of code to make it less readable.

One problem is with method chaining. We have a few different

factory

APIs

that encourage method chaining, and it put all the method

calls

on

a

single

line. For example here's one change:

-ClientCacheFactory ccf = new ClientCacheFactory()
-
.addPoolServer(NetworkUtils.getServerHostName(server1.

getHost()),

port)

- .set(SECURITY_CLIENT_AUTH_INIT,
UserPasswordAuthInit.class.getName() + ".create")
-.set(SECURITY_PREFIX+"username", "root")
-.set(SECURITY_PREFIX+"password", "root");
+

Re: Coding practices/standards

[Jared Stewart]

Fantastic, thanks for merging this in Mark.  For anyone with outstanding work 
on branches made before this change, your life may be made easier by 
cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added Spotless) 
into your branch and then running ‘gradlew spotlessApply’ on it before 
attempting to merge into develop.

— Jared
> On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:
> 
> Thanks Jared for the suggestion of Spotless and follow-up work.
> 
> This is now completed and checked into develop. As this does touch many
> files, be prepared the next time you pull.
> 
> --Mark
> 
> 
> 
> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart  wrote:
> 
>> Done! :)
>> 
>> - Jared
>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
>>> 
>>> One more time! :)
>>> 
>>> Conflicting files
>>> geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java
>>> geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXWithVersionsDUnitTest.java
>>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/
>> PRTransactionDUnitTest.java
>>> 
>>> --Mark
>>> 
>>> On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
>> wrote:
>>> 
 I just pulled and rebased onto develop, and force pushed into the
>> existing
 pull request.  It should be clean to merge in now.
 
 Thanks,
 Jared
> On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> 
> I believe there is enough consensus here to check this into develop.
> 
> Jared, due to recent checkins into develop, can you update the pull
 request
> one more time? Trying to make this as clean as possible. I will check
 into
> develop after the update, unless someone else gets to it first.
> 
> All, can we hold checkins on develop until the new formatter is
>> applied?
> 
> Thanks,
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
>> wrote:
> 
>> +1
>> 
>>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
>> bschucha...@pivotal.io>
>> wrote:
>>> 
>>> +1
>>> 
>>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
 +1
 
 
 On 20/10/16 4:56 pm, Mark Bretl wrote:
> +1 as well...
> 
> - Pulled changes
> - Executed './gradlew clean build' and was successful.
> - Modified a couple of random files to test
> - Ran './gradlew clean build' again and failed expectedly
> - Ran './gradlew spotlessApply', task was successful
> - Ran './gradlew clean build' and succeeded
> 
> Great addition! As long as others are good with the formatter,
>> then I
>> am
> good.
> 
> --Mark
> 
> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
>> wrote:
> 
>> +1 I just added my approval to the PR (and again here)
>> 
>> 
>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
>> jstew...@pivotal.io
> 
>> wrote:
>> 
>>> I have opened a pull request here >> incubator-geode/pull/268> to enable the Spotless plugin and to
>> switch to
>>> the Google Java Style formatter templates.
>>> 
>>> 
 On Oct 18, 2016, at 4:32 PM, Kirk Lund 
>> wrote:
 
 For reference TRAC #38741 was a bug with the summary
>> "EOFException
>> during
 deserialize on client update with copy-on-read=true"
 
 -Kirk
 
 
 On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
 jstew...@pivotal.io
>>> 
>>> wrote:
> To give everyone an update, using the Google Java Style eclipse
>> template
> there is an issue spotlessCheck where fails for
> geode-core/src/test/java/org/apache/geode/cache30/
>>> Bug38741DUnitTest.java
> even if you run it directly after spotlessApply. This needs to
>> be
> investigated and fixed before I can open a pull request to
>> enable
>>> spotless.
> 
>> On Oct 14, 2016, at 4:57 PM, Dan Smith 
 wrote:
>> 
>> +1 - The formatting looks better now.
>> 
>> -Dan
>> 
>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
>> jstew...@pivotal.io
> wrote:
>>> I agree that the formatter needs fixing up.  Our wiki <
>>> https://cwiki.apache.org/confluence/display/GEODE/Code+
>> Style+Guide>
> says
>>> that we follow the Google Java Style guide, but that is not
>> actually
> what’s
>>> in our formatter templates.  I pushed a new branch <

Re: Coding practices/standards

[Mark Bretl]

Thanks Jared for the suggestion of Spotless and follow-up work.

This is now completed and checked into develop. As this does touch many
files, be prepared the next time you pull.

--Mark



On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart  wrote:

> Done! :)
>
> - Jared
> > On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
> >
> > One more time! :)
> >
> > Conflicting files
> > geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java
> > geode-core/src/test/java/org/apache/geode/disttx/
> PRDistTXWithVersionsDUnitTest.java
> > geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> PRTransactionDUnitTest.java
> >
> > --Mark
> >
> > On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
> wrote:
> >
> >> I just pulled and rebased onto develop, and force pushed into the
> existing
> >> pull request.  It should be clean to merge in now.
> >>
> >> Thanks,
> >> Jared
> >>> On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> >>>
> >>> I believe there is enough consensus here to check this into develop.
> >>>
> >>> Jared, due to recent checkins into develop, can you update the pull
> >> request
> >>> one more time? Trying to make this as clean as possible. I will check
> >> into
> >>> develop after the update, unless someone else gets to it first.
> >>>
> >>> All, can we hold checkins on develop until the new formatter is
> applied?
> >>>
> >>> Thanks,
> >>>
> >>> --Mark
> >>>
> >>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> wrote:
> >>>
>  +1
> 
> > On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> bschucha...@pivotal.io>
>  wrote:
> >
> > +1
> >
> > Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> >> +1
> >>
> >>
> >> On 20/10/16 4:56 pm, Mark Bretl wrote:
> >>> +1 as well...
> >>>
> >>> - Pulled changes
> >>> - Executed './gradlew clean build' and was successful.
> >>> - Modified a couple of random files to test
> >>> - Ran './gradlew clean build' again and failed expectedly
> >>> - Ran './gradlew spotlessApply', task was successful
> >>> - Ran './gradlew clean build' and succeeded
> >>>
> >>> Great addition! As long as others are good with the formatter,
> then I
>  am
> >>> good.
> >>>
> >>> --Mark
> >>>
> >>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> wrote:
> >>>
>  +1 I just added my approval to the PR (and again here)
> 
> 
>  On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> jstew...@pivotal.io
> >>>
>  wrote:
> 
> > I have opened a pull request here  > incubator-geode/pull/268> to enable the Spotless plugin and to
>  switch to
> > the Google Java Style formatter templates.
> >
> >
> >> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
> wrote:
> >>
> >> For reference TRAC #38741 was a bug with the summary
> "EOFException
>  during
> >> deserialize on client update with copy-on-read=true"
> >>
> >> -Kirk
> >>
> >>
> >> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> >> jstew...@pivotal.io
> >
> > wrote:
> >>> To give everyone an update, using the Google Java Style eclipse
>  template
> >>> there is an issue spotlessCheck where fails for
> >>> geode-core/src/test/java/org/apache/geode/cache30/
> > Bug38741DUnitTest.java
> >>> even if you run it directly after spotlessApply. This needs to
> be
> >>> investigated and fixed before I can open a pull request to
> enable
> > spotless.
> >>>
>  On Oct 14, 2016, at 4:57 PM, Dan Smith 
> >> wrote:
> 
>  +1 - The formatting looks better now.
> 
>  -Dan
> 
>  On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
>  jstew...@pivotal.io
> >>> wrote:
> > I agree that the formatter needs fixing up.  Our wiki <
> > https://cwiki.apache.org/confluence/display/GEODE/Code+
>  Style+Guide>
> >>> says
> > that we follow the Google Java Style guide, but that is not
>  actually
> >>> what’s
> > in our formatter templates.  I pushed a new branch <
> > https://github.com/
> > jaredjstewart/incubator-geode/tree/
> spotlessPluginGoogleStyle>
>  that
> >>> points
> > spotless at the actual Google Java Style template, and I
> think
> >> it
> > makes
> > both of the examples you found look better.
> >
> >> On Oct 13, 2016, at 10:11 AM, Dan Smith 
>  wrote:
> >>
> >> +1 for adding this to ./gradlew build
> 

Re: Coding practices/standards

[Mark Bretl]

One more time! :)

Conflicting files
geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java
geode-core/src/test/java/org/apache/geode/disttx/PRDistTXWithVersionsDUnitTest.java
geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java

--Mark

On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart  wrote:

> I just pulled and rebased onto develop, and force pushed into the existing
> pull request.  It should be clean to merge in now.
>
> Thanks,
> Jared
> > On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> >
> > I believe there is enough consensus here to check this into develop.
> >
> > Jared, due to recent checkins into develop, can you update the pull
> request
> > one more time? Trying to make this as clean as possible. I will check
> into
> > develop after the update, unless someone else gets to it first.
> >
> > All, can we hold checkins on develop until the new formatter is applied?
> >
> > Thanks,
> >
> > --Mark
> >
> > On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe  wrote:
> >
> >> +1
> >>
> >>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt 
> >> wrote:
> >>>
> >>> +1
> >>>
> >>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
>  +1
> 
> 
>  On 20/10/16 4:56 pm, Mark Bretl wrote:
> > +1 as well...
> >
> > - Pulled changes
> > - Executed './gradlew clean build' and was successful.
> > - Modified a couple of random files to test
> > - Ran './gradlew clean build' again and failed expectedly
> > - Ran './gradlew spotlessApply', task was successful
> > - Ran './gradlew clean build' and succeeded
> >
> > Great addition! As long as others are good with the formatter, then I
> >> am
> > good.
> >
> > --Mark
> >
> > On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:
> >
> >> +1 I just added my approval to the PR (and again here)
> >>
> >>
> >> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart  >
> >> wrote:
> >>
> >>> I have opened a pull request here  >>> incubator-geode/pull/268> to enable the Spotless plugin and to
> >> switch to
> >>> the Google Java Style formatter templates.
> >>>
> >>>
>  On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
> 
>  For reference TRAC #38741 was a bug with the summary "EOFException
> >> during
>  deserialize on client update with copy-on-read=true"
> 
>  -Kirk
> 
> 
>  On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> jstew...@pivotal.io
> >>>
> >>> wrote:
> > To give everyone an update, using the Google Java Style eclipse
> >> template
> > there is an issue spotlessCheck where fails for
> > geode-core/src/test/java/org/apache/geode/cache30/
> >>> Bug38741DUnitTest.java
> > even if you run it directly after spotlessApply. This needs to be
> > investigated and fixed before I can open a pull request to enable
> >>> spotless.
> >
> >> On Oct 14, 2016, at 4:57 PM, Dan Smith 
> wrote:
> >>
> >> +1 - The formatting looks better now.
> >>
> >> -Dan
> >>
> >> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
> >> jstew...@pivotal.io
> > wrote:
> >>> I agree that the formatter needs fixing up.  Our wiki <
> >>> https://cwiki.apache.org/confluence/display/GEODE/Code+
> >> Style+Guide>
> > says
> >>> that we follow the Google Java Style guide, but that is not
> >> actually
> > what’s
> >>> in our formatter templates.  I pushed a new branch <
> >>> https://github.com/
> >>> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle>
> >> that
> > points
> >>> spotless at the actual Google Java Style template, and I think
> it
> >>> makes
> >>> both of the examples you found look better.
> >>>
>  On Oct 13, 2016, at 10:11 AM, Dan Smith 
> >> wrote:
> 
>  +1 for adding this to ./gradlew build
> 
>  But I think we might want to fix up the formatter a bit before
> >>> reformatting
>  the code. I tried running spotlessApply, and it did some
> >> unfortunate
>  reformatting of code to make it less readable.
> 
>  One problem is with method chaining. We have a few different
> >> factory
> > APIs
>  that encourage method chaining, and it put all the method
> calls
> >> on
> >> a
> >>> single
>  line. For example here's one change:
> 
>  -ClientCacheFactory ccf = new ClientCacheFactory()
>  -
>  

[GitHub] incubator-geode issue #267: Feature/geode 2015

[davebarnes97]

Github user davebarnes97 commented on the issue:

https://github.com/apache/incubator-geode/pull/267
  
Correction - bookbinder watch did not work reliably for me, so this PR does 
not introduce a new problem.
Re-verified that local bind yields good output.

IMO, OK to accept this PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

[kjduling]

Github user kjduling commented on a diff in the pull request:

https://github.com/apache/incubator-geode/pull/265#discussion_r84528366
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.geode.rest.internal.web;
+
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.json.JSONObject;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.i18n.LocalizedStrings;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+
+public class SwaggerVerificationTest extends AbstractRestTest {
+
+  @Test
+  public void isSwaggerRunning() throws Exception {
+// Check the UI
+HttpResponse response = doRequest(new 
HttpGet("/geode/swagger-ui.html"), "", "");
+assertThat(getCode(response), is(200));
+
+// Check the JSON
+response = doRequest(new HttpGet("/geode/v2/api-docs"), "", "");
--- End diff --

"geode/v2/api-docs" is the endpoint for Swagger2 documentation.  Swagger 
was "geode/api-docs".  Geode's endpoints are all "geode/v1/*" and I've verified 
that if/when the REST API revision is bumped to "v2", there will be no conflict 
unless we create a new endpoint called "v2/api-docs".

This is supposed to be able to be overridden with the 
"springfox.documentation.swagger.v2.path" env property, but it doesn't seem to 
be honored.  Or SwaggerConfig is being loaded too late in the lifecycle, which 
I doubt.  At any rate, I am adding the structure to use this, but leaving the 
values set to the default ones.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Coding practices/standards

[Jared Stewart]

I just pulled and rebased onto develop, and force pushed into the existing pull 
request.  It should be clean to merge in now.

Thanks,
Jared
> On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> 
> I believe there is enough consensus here to check this into develop.
> 
> Jared, due to recent checkins into develop, can you update the pull request
> one more time? Trying to make this as clean as possible. I will check into
> develop after the update, unless someone else gets to it first.
> 
> All, can we hold checkins on develop until the new formatter is applied?
> 
> Thanks,
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe  wrote:
> 
>> +1
>> 
>>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt 
>> wrote:
>>> 
>>> +1
>>> 
>>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
 +1
 
 
 On 20/10/16 4:56 pm, Mark Bretl wrote:
> +1 as well...
> 
> - Pulled changes
> - Executed './gradlew clean build' and was successful.
> - Modified a couple of random files to test
> - Ran './gradlew clean build' again and failed expectedly
> - Ran './gradlew spotlessApply', task was successful
> - Ran './gradlew clean build' and succeeded
> 
> Great addition! As long as others are good with the formatter, then I
>> am
> good.
> 
> --Mark
> 
> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:
> 
>> +1 I just added my approval to the PR (and again here)
>> 
>> 
>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
>> wrote:
>> 
>>> I have opened a pull request here >> incubator-geode/pull/268> to enable the Spotless plugin and to
>> switch to
>>> the Google Java Style formatter templates.
>>> 
>>> 
 On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
 
 For reference TRAC #38741 was a bug with the summary "EOFException
>> during
 deserialize on client update with copy-on-read=true"
 
 -Kirk
 
 
 On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart >> 
>>> wrote:
> To give everyone an update, using the Google Java Style eclipse
>> template
> there is an issue spotlessCheck where fails for
> geode-core/src/test/java/org/apache/geode/cache30/
>>> Bug38741DUnitTest.java
> even if you run it directly after spotlessApply. This needs to be
> investigated and fixed before I can open a pull request to enable
>>> spotless.
> 
>> On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
>> 
>> +1 - The formatting looks better now.
>> 
>> -Dan
>> 
>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
>> jstew...@pivotal.io
> wrote:
>>> I agree that the formatter needs fixing up.  Our wiki <
>>> https://cwiki.apache.org/confluence/display/GEODE/Code+
>> Style+Guide>
> says
>>> that we follow the Google Java Style guide, but that is not
>> actually
> what’s
>>> in our formatter templates.  I pushed a new branch <
>>> https://github.com/
>>> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle>
>> that
> points
>>> spotless at the actual Google Java Style template, and I think it
>>> makes
>>> both of the examples you found look better.
>>> 
 On Oct 13, 2016, at 10:11 AM, Dan Smith 
>> wrote:
 
 +1 for adding this to ./gradlew build
 
 But I think we might want to fix up the formatter a bit before
>>> reformatting
 the code. I tried running spotlessApply, and it did some
>> unfortunate
 reformatting of code to make it less readable.
 
 One problem is with method chaining. We have a few different
>> factory
> APIs
 that encourage method chaining, and it put all the method calls
>> on
>> a
>>> single
 line. For example here's one change:
 
 -ClientCacheFactory ccf = new ClientCacheFactory()
 -
 .addPoolServer(NetworkUtils.getServerHostName(server1.
>> getHost()),
> port)
 - .set(SECURITY_CLIENT_AUTH_INIT,
 UserPasswordAuthInit.class.getName() + ".create")
 -.set(SECURITY_PREFIX+"username", "root")
 -.set(SECURITY_PREFIX+"password", "root");
 +ClientCacheFactory ccf = new
 ClientCacheFactory().addPoolServer(NetworkUtils.
>>> getServerHostName(server1.getHost()),
 port).set(SECURITY_CLIENT_AUTH_INIT,
>> UserPasswordAuthInit.class.
> getName()
>>> +
 

Re: Coding practices/standards

[Anthony Baker]

Sounds like a plan!  Let’s do this :-)

Anthony

> On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> 
> I believe there is enough consensus here to check this into develop.
> 
> Jared, due to recent checkins into develop, can you update the pull request
> one more time? Trying to make this as clean as possible. I will check into
> develop after the update, unless someone else gets to it first.
> 
> All, can we hold checkins on develop until the new formatter is applied?
> 
> Thanks,
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe  wrote:
> 
>> +1
>> 
>>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt 
>> wrote:
>>> 
>>> +1
>>> 
>>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
 +1
 
 
 On 20/10/16 4:56 pm, Mark Bretl wrote:
> +1 as well...
> 
> - Pulled changes
> - Executed './gradlew clean build' and was successful.
> - Modified a couple of random files to test
> - Ran './gradlew clean build' again and failed expectedly
> - Ran './gradlew spotlessApply', task was successful
> - Ran './gradlew clean build' and succeeded
> 
> Great addition! As long as others are good with the formatter, then I
>> am
> good.
> 
> --Mark
> 
> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:
> 
>> +1 I just added my approval to the PR (and again here)
>> 
>> 
>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
>> wrote:
>> 
>>> I have opened a pull request here >> incubator-geode/pull/268> to enable the Spotless plugin and to
>> switch to
>>> the Google Java Style formatter templates.
>>> 
>>> 
 On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
 
 For reference TRAC #38741 was a bug with the summary "EOFException
>> during
 deserialize on client update with copy-on-read=true"
 
 -Kirk
 
 
 On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart >> 
>>> wrote:
> To give everyone an update, using the Google Java Style eclipse
>> template
> there is an issue spotlessCheck where fails for
> geode-core/src/test/java/org/apache/geode/cache30/
>>> Bug38741DUnitTest.java
> even if you run it directly after spotlessApply. This needs to be
> investigated and fixed before I can open a pull request to enable
>>> spotless.
> 
>> On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
>> 
>> +1 - The formatting looks better now.
>> 
>> -Dan
>> 
>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
>> jstew...@pivotal.io
> wrote:
>>> I agree that the formatter needs fixing up.  Our wiki <
>>> https://cwiki.apache.org/confluence/display/GEODE/Code+
>> Style+Guide>
> says
>>> that we follow the Google Java Style guide, but that is not
>> actually
> what’s
>>> in our formatter templates.  I pushed a new branch <
>>> https://github.com/
>>> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle>
>> that
> points
>>> spotless at the actual Google Java Style template, and I think it
>>> makes
>>> both of the examples you found look better.
>>> 
 On Oct 13, 2016, at 10:11 AM, Dan Smith 
>> wrote:
 
 +1 for adding this to ./gradlew build
 
 But I think we might want to fix up the formatter a bit before
>>> reformatting
 the code. I tried running spotlessApply, and it did some
>> unfortunate
 reformatting of code to make it less readable.
 
 One problem is with method chaining. We have a few different
>> factory
> APIs
 that encourage method chaining, and it put all the method calls
>> on
>> a
>>> single
 line. For example here's one change:
 
 -ClientCacheFactory ccf = new ClientCacheFactory()
 -
 .addPoolServer(NetworkUtils.getServerHostName(server1.
>> getHost()),
> port)
 - .set(SECURITY_CLIENT_AUTH_INIT,
 UserPasswordAuthInit.class.getName() + ".create")
 -.set(SECURITY_PREFIX+"username", "root")
 -.set(SECURITY_PREFIX+"password", "root");
 +ClientCacheFactory ccf = new
 ClientCacheFactory().addPoolServer(NetworkUtils.
>>> getServerHostName(server1.getHost()),
 port).set(SECURITY_CLIENT_AUTH_INIT,
>> UserPasswordAuthInit.class.
> getName()
>>> +
 ".create").set(SECURITY_PREFIX + "username",
> "root").set(SECURITY_PREFIX
>>> +

Re: Review Request 53092: GEODE-2011 Client clears pdx registry needs synchronization

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53092/
---

(Updated Oct. 21, 2016, 6:52 p.m.)


Review request for geode, Bruce Schuchardt, Udo Kohlmeyer, and Dan Smith.


Changes
---

Updated diff


Repository: geode


Description
---

We clear pdx registry in client when we re-connect to cluster. But during that 
time other thread may end up using old registry while other thread is clearing 
this. Thus we need to synchronize that. In current code it happens through 
listener events and I modified that notification to synchronize this.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java
 3f3d725 

Diff: https://reviews.apache.org/r/53092/diff/


Testing
---


Thanks,

Hitesh Khamesra

Review Request 53094: GEODE-706 race condition between expiry thread and other user thread.

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53094/
---

Review request for geode, Bruce Schuchardt, Darrel Schneider, and Udo Kohlmeyer.


Repository: geode


Description
---

ExpirationManager tracks the regionEntry as reference to expiryTask. It 
assumes, it is unique for that key. But consistency check mechanism keeps that 
regionEntry around and that enforces re-use of that for new update. That 
introduces the race condition between expiry thread and user thread, it endup 
not scheduling the entry for expiration. As a fix, now "consistency check 
mechanism" unschedules the expiry task to avoid this race condition.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java 
e02c7e1 
  geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java 
d059aab 
  geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java 
0c20d32 
  geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
ac4c705 

Diff: https://reviews.apache.org/r/53094/diff/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 53092: GEODE-2011 Client clears pdx registry needs synchronization

[Darrel Schneider]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53092/#review153572
---




geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java
 (line 74)


this code finds that it currently has no endpoint (endpoint == null) or the 
one it had has been closed (endpoint.isClosed). In both those cases you have it 
call listener.clearPdxRegistry which then calls 
PdxRegistryRecoveryListener.endPointNowInUse.

The does not make logical sense to me.
The endpoint that is now in use would be the one created on line 76, the 
new endpoint you are adding. The PdxRegistryRecoveryListener does not even use 
the "endpoint" argument but I wonder if you should be doing this on line 77 
with the new endpoint.


- Darrel Schneider


On Oct. 21, 2016, 10:57 a.m., Hitesh Khamesra wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53092/
> ---
> 
> (Updated Oct. 21, 2016, 10:57 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Udo Kohlmeyer, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> We clear pdx registry in client when we re-connect to cluster. But during 
> that time other thread may end up using old registry while other thread is 
> clearing this. Thus we need to synchronize that. In current code it happens 
> through listener events and I modified that notification to synchronize this.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java
>  3f3d725 
> 
> Diff: https://reviews.apache.org/r/53092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hitesh Khamesra
> 
>

Review Request 53093: GEODE-2025: do not use 8080 as the default http-server-port since we defined default in DistributionConfig

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53093/
---

Review request for geode.


Repository: geode


Description
---

GEODE-2025: do not use 8080 as the default http-server-port since we defined 
default in DistributionConfig

When we pulled in a pull request in Auguest, we didn't notice that the default 
http-service-port it's using is 8080. In DistributionConfig, we defined it to 
be 7070. This would cause a discrepency between the default http port we are 
using between locators and servers. change this to 7070 now.


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 c83cebb745ff931e28b81781a53952baf9465bd1 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
 5f66f3bd48055cd0b65726710d36c4fac1d30b40 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java
 PRE-CREATION 
  geode-core/src/main/java/org/apache/geode/cache/server/CacheServer.java 
910f34d448a27497a541793d8a6c260e56970d00 
  
geode-core/src/main/java/org/apache/geode/internal/cache/execute/util/FindRestEnabledServersFunction.java
 793f73c8cae62188815cf29fb98da8148cb140a8 
  geode-core/src/main/java/org/apache/geode/management/internal/RestAgent.java 
6e7ba7ddd9c21532c1c4978b6af81012a482b782 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java
 892a92d688a88de5742e70f28ff5442358a3b218 

Diff: https://reviews.apache.org/r/53093/diff/


Testing
---


Thanks,

Jinmei Liao

Review Request 53092: GEODE-2011 Client clears pdx registry needs synchronization

[Hitesh Khamesra]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53092/
---

Review request for geode, Bruce Schuchardt, Udo Kohlmeyer, and Dan Smith.


Repository: geode


Description
---

We clear pdx registry in client when we re-connect to cluster. But during that 
time other thread may end up using old registry while other thread is clearing 
this. Thus we need to synchronize that. In current code it happens through 
listener events and I modified that notification to synchronize this.


Diffs
-

  
geode-core/src/main/java/org/apache/geode/cache/client/internal/EndpointManagerImpl.java
 3f3d725 

Diff: https://reviews.apache.org/r/53092/diff/


Testing
---


Thanks,

Hitesh Khamesra

Re: Review Request 53071: GEODE-2021: Get on non colocated keys in a transaction does not throw TransactionDataNotColocatedException

[anilkumar gingade]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53071/#review153568
---


Ship it!




Ship It!

- anilkumar gingade


On Oct. 21, 2016, 5:08 p.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53071/
> ---
> 
> (Updated Oct. 21, 2016, 5:08 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Swapnil Bawaskar.
> 
> 
> Bugs: GEODE-2021
> https://issues.apache.org/jira/browse/GEODE-2021
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Throw TransactionDataNotColocatedException when get locally failed with 
> BucketNotFoundException
> Added a dunit test which passes with the fix and failed without the fix.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
>  baab79f 
>   geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java 
> f36085b 
>   
> geode-core/src/test/java/org/apache/geode/disttx/PRDistTXWithVersionsDUnitTest.java
>  268c2ed 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRColocationDUnitTest.java
>  1b8d2d1 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
>  516c240 
> 
> Diff: https://reviews.apache.org/r/53071/diff/
> 
> 
> Testing
> ---
> 
> precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>

Re: Review Request 53079: GEODE-2020: for rest api get request, use utf-8 as response encoding.

[Kevin Duling]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53079/#review153565
---


Ship it!




Ship It!

- Kevin Duling


On Oct. 21, 2016, 7:29 a.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53079/
> ---
> 
> (Updated Oct. 21, 2016, 7:29 a.m.)
> 
> 
> Review request for geode, Kevin Duling and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * add more test assertions.
> * fix legacy tests
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
>  PRE-CREATION 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
>  6e91894c2c50e21c1fa250330119a7013f6b0fa5 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
>  30c8b3aa160648cae2ae2176e746ff987aba3aec 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
>  e1ea1ade2e8633f983f60fcf3cb9b789cd9dc2f4 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
>  ebb8ccc08fe1766d278eaeaf053365ad0e9afcfb 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
>  d13c99c11151355f3595718b15505db779d4161e 
> 
> Diff: https://reviews.apache.org/r/53079/diff/
> 
> 
> Testing
> ---
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 53071: GEODE-2021: Get on non colocated keys in a transaction does not throw TransactionDataNotColocatedException

[Eric Shu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53071/
---

(Updated Oct. 21, 2016, 5:08 p.m.)


Review request for geode, Darrel Schneider and Swapnil Bawaskar.


Changes
---

Refactoring test based on review comments.


Bugs: GEODE-2021
https://issues.apache.org/jira/browse/GEODE-2021


Repository: geode


Description
---

Throw TransactionDataNotColocatedException when get locally failed with 
BucketNotFoundException
Added a dunit test which passes with the fix and failed without the fix.


Diffs (updated)
-

  
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java 
baab79f 
  geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java 
f36085b 
  
geode-core/src/test/java/org/apache/geode/disttx/PRDistTXWithVersionsDUnitTest.java
 268c2ed 
  
geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRColocationDUnitTest.java
 1b8d2d1 
  
geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java
 516c240 

Diff: https://reviews.apache.org/r/53071/diff/


Testing
---

precheckin.


Thanks,

Eric Shu

Re: C++ Coding Standards

[Udo Kohlmeyer]

+1 - I'm not a C++ dev but I'm all for consistency.


On 21/10/16 8:43 am, Jacob Barrett wrote:

Considering the team just formally adopted some standards and enforcement
for the Java sources I would like to open the discussion for formally
adopting similar standards for the C++ sources.

I propose that we use the Google C++ style as defined by
https://google.github.io/styleguide/cppguide.html and that such format
shall be enforced with clang-format -style=Google.

-Jake

Re: Coding practices/standards

[Kenneth Howe]

+1

> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt  wrote:
> 
> +1
> 
> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
>> +1
>> 
>> 
>> On 20/10/16 4:56 pm, Mark Bretl wrote:
>>> +1 as well...
>>> 
>>> - Pulled changes
>>> - Executed './gradlew clean build' and was successful.
>>> - Modified a couple of random files to test
>>> - Ran './gradlew clean build' again and failed expectedly
>>> - Ran './gradlew spotlessApply', task was successful
>>> - Ran './gradlew clean build' and succeeded
>>> 
>>> Great addition! As long as others are good with the formatter, then I am
>>> good.
>>> 
>>> --Mark
>>> 
>>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:
>>> 
 +1 I just added my approval to the PR (and again here)
 
 
 On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
 wrote:
 
> I have opened a pull request here  incubator-geode/pull/268> to enable the Spotless plugin and to switch to
> the Google Java Style formatter templates.
> 
> 
>> On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
>> 
>> For reference TRAC #38741 was a bug with the summary "EOFException
 during
>> deserialize on client update with copy-on-read=true"
>> 
>> -Kirk
>> 
>> 
>> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart 
> wrote:
>>> To give everyone an update, using the Google Java Style eclipse
 template
>>> there is an issue spotlessCheck where fails for
>>> geode-core/src/test/java/org/apache/geode/cache30/
> Bug38741DUnitTest.java
>>> even if you run it directly after spotlessApply. This needs to be
>>> investigated and fixed before I can open a pull request to enable
> spotless.
>>> 
 On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
 
 +1 - The formatting looks better now.
 
 -Dan
 
 On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart >> wrote:
> I agree that the formatter needs fixing up.  Our wiki <
> https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide> 
>>> says
> that we follow the Google Java Style guide, but that is not actually
>>> what’s
> in our formatter templates.  I pushed a new branch <
> https://github.com/
> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that
>>> points
> spotless at the actual Google Java Style template, and I think it
> makes
> both of the examples you found look better.
> 
>> On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:
>> 
>> +1 for adding this to ./gradlew build
>> 
>> But I think we might want to fix up the formatter a bit before
> reformatting
>> the code. I tried running spotlessApply, and it did some
 unfortunate
>> reformatting of code to make it less readable.
>> 
>> One problem is with method chaining. We have a few different
 factory
>>> APIs
>> that encourage method chaining, and it put all the method calls on
 a
> single
>> line. For example here's one change:
>> 
>> -ClientCacheFactory ccf = new ClientCacheFactory()
>> -
>> .addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),
>>> port)
>> - .set(SECURITY_CLIENT_AUTH_INIT,
>> UserPasswordAuthInit.class.getName() + ".create")
>> -.set(SECURITY_PREFIX+"username", "root")
>> -.set(SECURITY_PREFIX+"password", "root");
>> +ClientCacheFactory ccf = new
>> ClientCacheFactory().addPoolServer(NetworkUtils.
> getServerHostName(server1.getHost()),
>> port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.
>>> getName()
> +
>> ".create").set(SECURITY_PREFIX + "username",
>>> "root").set(SECURITY_PREFIX
> +
>> "password", "root");
>> 
>> I see a similar problem where it put array initialization all on a
>>> single
>> line:
>> 
>> +  public void testMultiColOrderByWithIndexResultWithProjection()
>>> throws
>> Exception {
>>   String queries[] = {
>>   // Test case No. IUMR021
>> -"SELECT   ID, description, createTime, pkid FROM
 /portfolio1
>>> pf1
>> where ID > 10 order by ID desc, pkid desc ",
>> -"SELECT   ID, description, createTime, pkid FROM
 /portfolio1
>>> pf1
>> where ID > 10 order by ID asc, pkid asc ",
>> -"SELECT   ID, description, createTime, pkid FROM
 /portfolio1
>>> pf1
>> where ID > 10 and ID < 20 order by ID asc, pkid asc ",
>> -"SELECT   ID, 

Re: Coding practices/standards

[Bruce Schuchardt]

+1

Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :

+1


On 20/10/16 4:56 pm, Mark Bretl wrote:

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the formatter, then I am
good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:


+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
wrote:


I have opened a pull request here  to enable the Spotless plugin and to 
switch to

the Google Java Style formatter templates.



On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:

For reference TRAC #38741 was a bug with the summary "EOFException

during

deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart 

wrote:

To give everyone an update, using the Google Java Style eclipse

template

there is an issue spotlessCheck where fails for
geode-core/src/test/java/org/apache/geode/cache30/

Bug38741DUnitTest.java

even if you run it directly after spotlessApply. This needs to be
investigated and fixed before I can open a pull request to enable

spotless.



On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:

+1 - The formatting looks better now.

-Dan

On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart 
 


says
that we follow the Google Java Style guide, but that is not 
actually

what’s

in our formatter templates.  I pushed a new branch <

https://github.com/

jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that

points

spotless at the actual Google Java Style template, and I think it

makes

both of the examples you found look better.

On Oct 13, 2016, at 10:11 AM, Dan Smith  
wrote:


+1 for adding this to ./gradlew build

But I think we might want to fix up the formatter a bit before

reformatting

the code. I tried running spotlessApply, and it did some

unfortunate

reformatting of code to make it less readable.

One problem is with method chaining. We have a few different

factory

APIs
that encourage method chaining, and it put all the method 
calls on

a

single

line. For example here's one change:

-ClientCacheFactory ccf = new ClientCacheFactory()
-
.addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),

port)

- .set(SECURITY_CLIENT_AUTH_INIT,
UserPasswordAuthInit.class.getName() + ".create")
-.set(SECURITY_PREFIX+"username", "root")
-.set(SECURITY_PREFIX+"password", "root");
+ClientCacheFactory ccf = new
ClientCacheFactory().addPoolServer(NetworkUtils.

getServerHostName(server1.getHost()),

port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.

getName()

+

".create").set(SECURITY_PREFIX + "username",

"root").set(SECURITY_PREFIX

+

"password", "root");

I see a similar problem where it put array initialization all 
on a

single

line:

+  public void testMultiColOrderByWithIndexResultWithProjection()

throws

Exception {
   String queries[] = {
   // Test case No. IUMR021
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID desc, pkid desc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID asc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID asc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID desc , pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID != 10 order by ID asc , pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID != 10 order by ID desc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID desc, pkid desc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID asc, pkid asc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
-"SELECT   ID, description, createTime, pkid 

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

[jinmeiliao]

Github user jinmeiliao commented on a diff in the pull request:

https://github.com/apache/incubator-geode/pull/265#discussion_r84500905
  
--- Diff: 
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java
 ---
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.geode.rest.internal.web;
+
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.json.JSONObject;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.i18n.LocalizedStrings;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+
+public class SwaggerVerificationTest extends AbstractRestTest {
+
+  @Test
+  public void isSwaggerRunning() throws Exception {
+// Check the UI
+HttpResponse response = doRequest(new 
HttpGet("/geode/swagger-ui.html"), "", "");
+assertThat(getCode(response), is(200));
+
+// Check the JSON
+response = doRequest(new HttpGet("/geode/v2/api-docs"), "", "");
--- End diff --

Just curious, why this is geode/v2, but all the api is still geode/v1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

[jinmeiliao]

Github user jinmeiliao commented on a diff in the pull request:

https://github.com/apache/incubator-geode/pull/265#discussion_r84501406
  
--- Diff: 
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
 ---
@@ -93,8 +93,7 @@ protected String getRestApiVersion() {
   response = void.class
   )
   @ApiResponses({
-  @ApiResponse(code = 200, message = "OK."),
-  @ApiResponse( code = 401, message = "Invalid Username or Password." 
),
+  @ApiResponse(code = 200, message = "OK."), @ApiResponse(code = 401, 
message = "Invalid Username or Password."),
--- End diff --

use the old fomat?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 53079: GEODE-2020: for rest api get request, use utf-8 as response encoding.

[Jinmei Liao]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53079/
---

(Updated Oct. 21, 2016, 2:29 p.m.)


Review request for geode, Kevin Duling and Kirk Lund.


Repository: geode


Description
---

* add more test assertions.
* fix legacy tests


Diffs
-

  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/GeodeRestClient.java
 PRE-CREATION 
  
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
 6e91894c2c50e21c1fa250330119a7013f6b0fa5 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java
 30c8b3aa160648cae2ae2176e746ff987aba3aec 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java
 e1ea1ade2e8633f983f60fcf3cb9b789cd9dc2f4 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
 ebb8ccc08fe1766d278eaeaf053365ad0e9afcfb 
  
geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
 d13c99c11151355f3595718b15505db779d4161e 

Diff: https://reviews.apache.org/r/53079/diff/


Testing (updated)
---

precheckin successful


Thanks,

Jinmei Liao