Re: Review Request 51750: GEODE-1873: Updating geode session module to support Tomcat 7.0.70

2016-09-12 Thread nabarun nag 

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


Ship it!




Ship It!

- nabarun nag


On Sept. 8, 2016, 10:30 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51750/
> ---
> 
> (Updated Sept. 8, 2016, 10:30 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, William Markito, and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There were changes in Tomcat that changed the field type of "attributes" from 
> Map to ConcurrentMap.  This breaks serialization of the DeltaSession object 
> and causes exceptions to be thrown.
> 
> This patch adds a new DeltaSession7 object that should fix this problem.
> 
> 
> Diffs
> -
> 
>   
> extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession7.java
>  PRE-CREATION 
>   
> extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat7DeltaSessionManager.java
>  8776c16 
>   
> extensions/geode-modules-tomcat7/src/test/java/com/gemstone/gemfire/modules/session/Tomcat7SessionsJUnitTest.java
>  e7970d7 
>   
> extensions/geode-modules/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsBase.java
>  5726013 
>   gradle/dependency-versions.properties a19520c 
> 
> Diff: https://reviews.apache.org/r/51750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Re: Review Request 51750: GEODE-1873: Updating geode session module to support Tomcat 7.0.70

2016-09-12 Thread nabarun nag 


> On Sept. 12, 2016, 6:59 p.m., nabarun nag wrote:
> > Ship It!

IntelliJ code inspect + running the tests using gradle


- nabarun


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


On Sept. 8, 2016, 10:30 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51750/
> ---
> 
> (Updated Sept. 8, 2016, 10:30 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, William Markito, and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There were changes in Tomcat that changed the field type of "attributes" from 
> Map to ConcurrentMap.  This breaks serialization of the DeltaSession object 
> and causes exceptions to be thrown.
> 
> This patch adds a new DeltaSession7 object that should fix this problem.
> 
> 
> Diffs
> -
> 
>   
> extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession7.java
>  PRE-CREATION 
>   
> extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat7DeltaSessionManager.java
>  8776c16 
>   
> extensions/geode-modules-tomcat7/src/test/java/com/gemstone/gemfire/modules/session/Tomcat7SessionsJUnitTest.java
>  e7970d7 
>   
> extensions/geode-modules/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsBase.java
>  5726013 
>   gradle/dependency-versions.properties a19520c 
> 
> Diff: https://reviews.apache.org/r/51750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Re: Review Request 51750: GEODE-1873: Updating geode session module to support Tomcat 7.0.70

2016-09-12 Thread nabarun nag 

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




extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession7.java
 (line 149)


//mgr.logCurrentStack(); 
Can we remove this comment?



extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession7.java
 (line 406)


Removable comment?



extensions/geode-modules-tomcat7/src/test/java/com/gemstone/gemfire/modules/session/Tomcat7SessionsJUnitTest.java
 (line 61)


Do we need check to see if the response was a success at this point?


- nabarun nag


On Sept. 8, 2016, 10:30 p.m., Jason Huynh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51750/
> ---
> 
> (Updated Sept. 8, 2016, 10:30 p.m.)
> 
> 
> Review request for geode, Barry Oglesby, William Markito, and nabarun nag.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> There were changes in Tomcat that changed the field type of "attributes" from 
> Map to ConcurrentMap.  This breaks serialization of the DeltaSession object 
> and causes exceptions to be thrown.
> 
> This patch adds a new DeltaSession7 object that should fix this problem.
> 
> 
> Diffs
> -
> 
>   
> extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession7.java
>  PRE-CREATION 
>   
> extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat7DeltaSessionManager.java
>  8776c16 
>   
> extensions/geode-modules-tomcat7/src/test/java/com/gemstone/gemfire/modules/session/Tomcat7SessionsJUnitTest.java
>  e7970d7 
>   
> extensions/geode-modules/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsBase.java
>  5726013 
>   gradle/dependency-versions.properties a19520c 
> 
> Diff: https://reviews.apache.org/r/51750/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>

Review Request 51750: GEODE-1873: Updating geode session module to support Tomcat 7.0.70

2016-09-08 Thread Jason Huynh 

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

Review request for geode, Barry Oglesby, William Markito, and nabarun nag.


Repository: geode


Description
---

There were changes in Tomcat that changed the field type of "attributes" from 
Map to ConcurrentMap.  This breaks serialization of the DeltaSession object and 
causes exceptions to be thrown.

This patch adds a new DeltaSession7 object that should fix this problem.


Diffs
-

  
extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/DeltaSession7.java
 PRE-CREATION 
  
extensions/geode-modules-tomcat7/src/main/java/com/gemstone/gemfire/modules/session/catalina/Tomcat7DeltaSessionManager.java
 8776c16 
  
extensions/geode-modules-tomcat7/src/test/java/com/gemstone/gemfire/modules/session/Tomcat7SessionsJUnitTest.java
 e7970d7 
  
extensions/geode-modules/src/test/java/com/gemstone/gemfire/modules/session/TestSessionsBase.java
 5726013 
  gradle/dependency-versions.properties a19520c 

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


Testing
---


Thanks,

Jason Huynh