Support Alternate Session Managers in Standalone War Bootstrap

[jbcpollak]

This PR adds the ability to specify a session manager factory on the command line within the standalone war. This can be used with any session manager that has a factory class with a zero-argument constructor.

Using this patch, in conjunction with a matching memcached-session-manager patch, you can use the memcached-session-manager with start your standalone project like this:

java -Dmsm.memcachedNodes="n1:localhost:21211" -jar standalone.jar \
   -sessionManagerFactory de.javakaffee.web.msm.MemcachedBackupSessionManagerFactory

The memcached-session-manager patch can be found here:

magro/memcached-session-manager#33

[magro]

You should keep commits for different pull requests in separate branches and don't mix'em up.

Your solution seems to be very specific to your specific needs. I've never looked into Tomcat7RunnerCli, but I'd look for some extension points or add them, so that a value is created that's useful for others as well. Then it's more likely that your patch will be accepted.

Because this is only a mirror of the official repo, do you think the PR is really noticed? (Also the last PR was merged 2 years ago, and there's one PR open since 10 months). Maybe you should see what's the "official" way to get changes into the code base.

[jbcpollak]

This is why I love the open source community.

You should keep commits for different pull requests in separate branches and don't mix'em up.

I absolutely agree, my mistake. I decided to see if this PR would generate any constructive critiques before cleaning it up, and the other modification is minor and constructive.

Your solution seems to be very specific to your specific needs. I've never looked into Tomcat7RunnerCli, but I'd look for some extension points or add them, so that a value is created that's useful for others as well. Then it's more likely that your patch will be accepted.

You've never looked at the code, yet you are venturing an opinion on the implementation? I did look at the code, and there is no current extension point. If you have a suggestion for a more Apache-like, or Tomcat-like method for adding extensions like this, I'd be happen to consider and implement it. As is, this was a simple way to get a proof of concept going and provide something that is useful to others: namely, a way to override the SessionManager in a flexible way.

Perhaps its not clear from the PR, but the MemcacheSessionManager is only one possible implementation, this PR would support any SessionManager that implements with a zero-args constructor factory class.

Because this is only a mirror of the official repo, do you think the PR is really noticed? (Also the last PR was merged 2 years ago, and there's one PR open since 10 months). Maybe you should see what's the "official" way to get changes into the code base.

I filed a bug and in the official Apache JIRA and it was triaged, so I think I've got that covered, but please let me know if you think I missed anything.

[jbcpollak]

For future posterity, here is the JIRA ticket: https://issues.apache.org/jira/browse/MTOMCAT-250

[magro]

You've never looked at the code, yet you are venturing an opinion on the
implementation? I /did/ look at the code, and there is no current
extension point.

It was not meant as an offense, I just wanted to share my impression.
I looked at your changes and I wondered if it would make sense to allow
to dynamically add options via some "plugin" system based on stuff found
in the classpath (you need to extend the classpath anyways).
This way you wouldn't need s.th. like a SessionFactory with a
createSessionManager method but you could it add dynamically. Remember
that the createSessionManager thing would need to get documented somewhere.

Basically I would try to add more general concepts that support more use
cases.

If you have a suggestion for a more Apache-like, or
Tomcat-like method for adding extensions like this, I'd be happen to
consider and implement it.

As said above, I'd try to add some Plugin concept that allows to add
options dynamically.

• Lookup Plugins from classpath
• Invoke s.th. like plugin.registerOption(options)
• Invoke s.th. like plugin.handleOptions(line, tomcat7Runner)

Not sure if it would really would work like this, just a rough idea.

But basically I think the most important thing is to get in touch with
the developers and check with them what's their preferred solution ;-)

I filed a bug and in the official Apache JIRA and it was triaged, so I
think I've got that covered.

I just voted for it :-)

[jbcpollak]

Hi @magro , first I want to apologize for being a bit sharp, I was caught off guard a bit. Based on how well structured and formalized the MSM code is, I know you are a very thorough engineer, so I respect the desire to make things as flexible and organized as possible.

In this specific case, I think the tomcat-standalone code is not written with such a formalized approach, and I get the impression it was written with an "I've got a specific use case" approach by its developers, so I took that approach for this patch.

If the maintainers want to take this feature request as a call for broadening the overall flexibility of the standalone code, thats fine, but I feel it is out of scope for this specific use case and no reason to delay integration of this the patch. I say that because I don't believe the style of the code implemented differs significantly from the existing code, not because I feel the existing code is as generalized as it could be. I don't consider it my place to take on that level of architectural refactoring.

For the proposed patch to the MSM code I feel differently, since I realize the patch IS out of character for the existing code.

[olamy]

we usually avoid using * import

[olamy]

pr merged.
You can close it
Thanks!

[olamy]

@jbcpollak could you please close the pr?
Thanks!

[jbcpollak]

Oops sorry!