Skip to content

[#145612229] add session state caching support for geode - #437

Closed
Jammy-Louie wants to merge 5 commits into
cloudfoundry:masterfrom
cf-gemfire-org:ssc-gemfire
Closed

[#145612229] add session state caching support for geode#437
Jammy-Louie wants to merge 5 commits into
cloudfoundry:masterfrom
cf-gemfire-org:ssc-gemfire

Conversation

@Jammy-Louie

Copy link
Copy Markdown

Pull request to Support Session State Caching for Pivotal Cloud Cache (Gemfire)

@cfdreddbot

Copy link
Copy Markdown

Hey pivotal-Jammy-Louie!

Thanks for submitting this pull request!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

Once you've publicized your membership, one of the owners of this repository can close and reopen this pull request, and dreddbot will take another look.

@nebhale nebhale self-assigned this Jun 2, 2017
@nebhale nebhale added this to the v3.18 milestone Jun 2, 2017

@nebhale nebhale left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before I even do a review of the code, this needs to be updated so as not to remove the contents of .idea/ and not exclude them. This information is intentionally kept under source control as it is a good indicator that your system is not setup in an authoritative way when it changes.

@Jammy-Louie

Copy link
Copy Markdown
Author

Updated the branch not to ignore .idea/ and include the infrastructural files

@nebhale nebhale left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the changes listed here (which are for the most part structurally sound), you'll also need to add some documentation to the Tomcat docs about this integration. This will probably be helpful.

Comment thread Gemfile.lock Outdated
addressable (2.5.1)
public_suffix (~> 2.0, >= 2.0.2)
ast (2.3.0)
coderay (1.1.1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should not be modified.

Comment thread config/tomcat.yml
connection_pool_size: 2
geode_store:
version: 0.+
repository_root: "https://tomcat-javabuildpack-spike.s3.amazonaws.com/geode-store" No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like a production version of this bucket URI. Also, for performance reasons, buckets hosted on S3 should probably exposed via CloudFront.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the process of find out where i can place the tar

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the code really is Geode, and is all backed by an ASL repository, we could host it for you.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all but one jar is from Geode

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is that one file from? Is it GemFire branded?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the jar is called pcc-client-auth

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it'll need to be a public repository with a permissive license for us to host it. This goes back to the fact that the CFF needs to take a special exception for US export compliance that can only be satisfied by 100% open source code.

app_dir = Pathname.new(File.expand_path(app_dir))
Logging::LoggerFactory.instance.setup app_dir
application = Component::Application.new(app_dir)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should not be removed.

Comment thread lib/java_buildpack/container/tomcat.rb Outdated
TomcatLoggingSupport.new(sub_configuration_context(context, 'logging_support')),
TomcatAccessLoggingSupport.new(sub_configuration_context(context, 'access_logging_support')),
TomcatRedisStore.new(sub_configuration_context(context, 'redis_store')),
TomcatGeodeStore.new(sub_configuration_context(context, 'geode_store')),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be sorted alphabetically.

Comment thread lib/java_buildpack/container/tomcat.rb Outdated
require 'java_buildpack/container'
require 'java_buildpack/container/tomcat/tomcat_insight_support'
require 'java_buildpack/container/tomcat/tomcat_instance'
require 'java_buildpack/container/tomcat/tomcat_geode_store'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be sorted alphabetically.

context 'when there is a session-replication service' do
before do
allow(services).to receive(:one_service?).with(/session-replication/, 'locators', 'users')
.and_return(true)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to run bundle exec rake to ensure that your code meets all formatting requirements. The Rubocop task will give you pointers on where the code needs to change.

require 'java_buildpack/container/tomcat'
require 'java_buildpack/container/tomcat/tomcat_insight_support'
require 'java_buildpack/container/tomcat/tomcat_instance'
require 'java_buildpack/container/tomcat/tomcat_geode_store'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetize

let(:configuration) do
{ 'tomcat' => tomcat_configuration,
'lifecycle_support' => lifecycle_support_configuration,
'geode_store' => geode_store_configuration,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetize

.to receive(:new).with(sub_configuration_context(tomcat_configuration))
allow(JavaBuildpack::Container::TomcatLifecycleSupport)
.to receive(:new).with(sub_configuration_context(lifecycle_support_configuration))
allow(JavaBuildpack::Container::TomcatGeodeStore)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetize


let(:logging_support_configuration) { instance_double('logging-support-configuration') }

let(:geode_store_configuration) { instance_double('geode_store_configuration') }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetize

@nebhale

nebhale commented Jun 2, 2017

Copy link
Copy Markdown
Contributor

@pivotal-Jammy-Louie Also noticed that you need to take care of publicizing your CLA compliance.

@nebhale

nebhale commented Jun 2, 2017

Copy link
Copy Markdown
Contributor

OK, looks like just the repository outstanding and then we can merge. Let's chat Monday morning about what the options are there.

@Jammy-Louie Jammy-Louie closed this Jun 5, 2017
@Jammy-Louie
Jammy-Louie deleted the ssc-gemfire branch June 5, 2017 15:40
@Jammy-Louie
Jammy-Louie restored the ssc-gemfire branch June 5, 2017 15:42
@nebhale nebhale reopened this Jun 5, 2017
@cfdreddbot

Copy link
Copy Markdown

Hey pivotal-Jammy-Louie!

Thanks for submitting this pull request!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

Once you've publicized your membership, one of the owners of this repository can close and reopen this pull request, and dreddbot will take another look.

@nebhale

nebhale commented Jun 12, 2017

Copy link
Copy Markdown
Contributor

@pivotal-Jammy-Louie Can you please publicize your org so that the CLA is satisfied?

@nebhale

nebhale commented Jun 19, 2017

Copy link
Copy Markdown
Contributor

@pivotal-Jammy-Louie We're still waiting on you to publicize your GitHub org membership to identify that you've signed the CLA.

@nebhale nebhale modified the milestones: v3.19, v3.18 Jun 19, 2017
@nebhale nebhale closed this Jul 4, 2017
@nebhale nebhale reopened this Jul 4, 2017
@cfdreddbot

Copy link
Copy Markdown

Hey pivotal-Jammy-Louie!

Thanks for submitting this pull request!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

Once you've publicized your membership, one of the owners of this repository can close and reopen this pull request, and dreddbot will take another look.

@Jammy-Louie

Copy link
Copy Markdown
Author

I have my Org. I apologize for the delay

@nebhale nebhale closed this Jul 6, 2017
@nebhale nebhale reopened this Jul 6, 2017
@cfdreddbot

Copy link
Copy Markdown

Hey pivotal-Jammy-Louie!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@nebhale

nebhale commented Jul 6, 2017

Copy link
Copy Markdown
Contributor

Great. The PR is more or less ready to go now, after we determine the hosting thing.

@pulkit-chandra

Copy link
Copy Markdown

I think this PR is ready to be merged, please let us know @nebhale if you have more feedback on it.

@nebhale

nebhale commented Jul 17, 2017

Copy link
Copy Markdown
Contributor

@nebhale nebhale closed this in 0e5a542 Jul 17, 2017
chook pushed a commit to takipi/java-buildpack that referenced this pull request Jul 23, 2017
This change adds support for the Pivotal Cloud Cache as a Tomcat Session
Manager.  Binding to a Pivotal Cloud Cache service instance will cause Tomcat
to use it as a session store.

[resolves cloudfoundry#437]
@luisapace

Copy link
Copy Markdown

@pivotal-Jammy-Louie
Hi Jammy,
I've had your name and this thread from Neb Hale, I kindly ask you if you can help me or redirect this to the responsible people.

I am an IBM employee working in legal team and my goal is to analyze with some software tools the source code and the license of all open sources and their dependencies to check if they can be included in our products without legal issues (in .cc a colleague working with me).

Currently I'm analyzing the java buildpack 3.19 code to verify if we can include it in our code without legal problems; to do that I've to analyze it and all its dependencies chain.

Now, the java buildpack 3.19 calls as dependency - with the name Apache Geode Tomcat Session Store - the zip file at:https://java-buildpack.cloudfoundry.org/geode-store/geode-store-0.0.2.tar.gz, that in turn include some jar files that seems to be NOT open source, but commercial code, Pivotal proprietary:
geode-core-9.0.3.jar
geode-modules-9.0.3.jar
geode-modules-tomcat8-9.0.3.jar
pcc-client-auth-0.0.1.jar

Could you please confirm me if they are commercial code or opensource?

If they are opensource could you please point me to the download location of their source.

Feel free to ask us other details if you need.

Thanks for your support.

ramonskie pushed a commit that referenced this pull request Dec 4, 2025
This change adds support for the Pivotal Cloud Cache as a Tomcat Session
Manager.  Binding to a Pivotal Cloud Cache service instance will cause Tomcat
to use it as a session store.

[resolves #437]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants