Skip to content
This repository has been archived by the owner on Feb 7, 2018. It is now read-only.

Java 7 and Tomcat 8 support + AWS SDK updated #17

Merged
merged 8 commits into from
Mar 11, 2015
Merged

Java 7 and Tomcat 8 support + AWS SDK updated #17

merged 8 commits into from
Mar 11, 2015

Conversation

darccio
Copy link
Contributor

@darccio darccio commented Dec 12, 2014

In order to keep this project working on the latest versions of Java and Tomcat's available packages, I created this pull request.

@johnthuss
Copy link

I'd really like to see Tomcat 8 support integrated.

However, this pull request includes a lot of unnecessary whitespace changes to these files:
src/main/java/com/amazonaws/services/dynamodb/sessionmanager/DynamoDBSessionStore.java
pom.xml

Revert the whitespace changes and resubmit and maybe it will actually have a chance of getting merged.

@darccio
Copy link
Contributor Author

darccio commented Dec 17, 2014

Done. I formatted the POM in the last commit on purpose to keep it in a better shape.

@darccio
Copy link
Contributor Author

darccio commented Dec 17, 2014

Hold this pull. I found some strange behavior that I would like to pin down before merging.

@darccio
Copy link
Contributor Author

darccio commented Jan 14, 2015

Ok, this pull is good to go, the strange behavoir was a quirk of our code on Java 8 (boxing/unboxing issues). It should be noted that it wasn't tested on Tomcat 7.

@hanshuo-aws
Copy link
Contributor

We have accepted pull request #16 which updates the SDK dependency.

For the Tomcat 8 changes, I believe they are not backwards-compatible with Tomcat 7, since getContext() was not added until Tomcat 8. We can't accept this change and then break all the Tomcat 7 users.
Should we instead create another branch for Tomcat 8 support? (and also use a different maven version there?)

@darccio
Copy link
Contributor Author

darccio commented Jan 30, 2015

You are right, Tomcat 8 changes are not backwards-compatible. I'm sorry to have introduced a non compatible modification. We should create another branch for Tomcat 8 support.

@hyandell
Copy link
Contributor

hyandell commented Feb 3, 2015

Wouldn't you create a Tomcat 7 legacy branch, that over time can be deprecated when Tomcat 7 reaches end of life?

@fulghum
Copy link
Contributor

fulghum commented Feb 3, 2015

Good suggestion Henri. We should spin up a tomcat-7 branch for the current code and move on to Tomcat 8 for the master branch.

@darccio
Copy link
Contributor Author

darccio commented Feb 4, 2015

👍 It is a really good idea.

@shorea
Copy link
Contributor

shorea commented Mar 10, 2015

We are accepting this pull request with a few modifications to preserve backwards compatibility with Tomcat 7. Thanks for the contribution!

@shorea shorea merged commit f079854 into amazon-archives:master Mar 11, 2015
shorea added a commit that referenced this pull request Mar 11, 2015
@shorea
Copy link
Contributor

shorea commented Mar 11, 2015

@darccio
Copy link
Contributor Author

darccio commented Mar 11, 2015

Thanks! Today I planned to do more Tomcat DynamoDB sessions' testing and your message arrived at the perfect time.

@Selindek
Copy link
Contributor

During you test try to modify some data in the stored session. After 30 seconds the changes will be lost....

@shorea
Copy link
Contributor

shorea commented Mar 11, 2015

Selindek, how consistenly are you seeing this behavior?

@shorea
Copy link
Contributor

shorea commented Mar 11, 2015

Tried it with the current and the previous release and seeing the issue sporadically in both. This seems to be the same issue pointed out in your pull request #19, correct?

@Selindek
Copy link
Contributor

Yes that's the same issue. But it happened continuously in our system. That's why I created my version. We use my version on a live server for more than a month and there are no any problems.
If you want to reproduce the issue for sure, do the following:
create a session
wait a minute to make sure the session is stored in the DB.
modify a data in the session.
wait at least 30 seconds. (Idle tests run in every 30 seconds by default)
... the old version will be loaded back from the DB

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants