-
Notifications
You must be signed in to change notification settings - Fork 41.2k
Consider reinstating auto-configuration for Spring Session Mongo DB #9552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
👍 Project's coordinates have changes (it's However note that we're strongly considering splitting the Spring Session into modules by session repository implementation (update: see spring-projects/spring-session#806), meaning there will be a This will require some changes in Boot's auto-config support for Spring Session so it might be a good idea to handle that change before reverting old MongoDB related code. /cc @rwinch |
@rwinch Can you please help us to understand the motivation for splitting out the Mongo DB support from Spring Session? We're curious about the benefits of splitting things up. Is jumping straight in and supporting it again in Spring Boot the right thing for us to do? Put another way, is it going to be maintained in the long term? |
@wilkinsona The background of these changes were explained in depth in spring-projects/spring-session#768 and https://spring.io/blog/2017/05/11/spring-session-2-0-0-m1-released. The new |
@wilkinsona I think this was explained quite well by @vpavic One thing to add is I view Spring Session as shaping to be a little like Spring Data team in that different data stores are owned by different teams. The data stores supported by the Spring Team are officially supported by their respective team and those that are not are community supported. |
@wilkinsona I'm all in favor of adding the managed version to Boot's listing of versions, yet holding off autoconfiguration until the dust settles. For example, I know Rob is working on core changes that haven't been picked up yet in Spring Session MongoDB. So if he and @vpavic need to hammer out some artifact details, by all means let them proceed. But in the meantime, let's not hold back people that want access to this officially supported product from picking up the right version a la Boot. Again, this is akin to Spring Data, so I didn't realize there was yet controversy over divvying up into separate repos. |
Thank you, @vpavic and @rwinch
Generally speaking, we only provide dependency management for something that Boot's using and won't just provide a version for something. I've subscribed to spring-projects/spring-session#806. Let's wait for that to land before we tackle this. |
@rwinch if that module is officially supported, why isn't it part of the bom? We shouldn't have a separate entry in dependency management for this IMO |
@snicoll BOM? Are you referring to the Spring IO platform BOM or to a Spring Session BOM that doesn't exist at this stage? |
There is a reference to a bom in the description of spring-projects/spring-session#806 |
There is no BOM right now. We're discussing the subject on Slack right now. |
@snicoll The description of that issue refers to previous state of the |
Now that the Spring Session project has been split up we're noticing an unfortunate side-effect with the Spring Boot support. With Spring Boot 1.5 the user would select "Spring Session" from http://start.spring.io and combine it with the technology that they wanted. For example, "Spring Session" + "Redis" would provide Redis Session support. This worked because the With the new layout, dependencies are no longer optional and the JAR is named after the technology being used. We've got a few options that we can consider, but none of them feel ideal: Have multiple "Session" starters on start.spring.ioWe could explode the existing "Spring Session" checkbox into N options ("Spring Session Redis", "Spring Session JDBC") etc. I'm not a big fan of this idea because it adds a lot of additional noise to the site. Add transitive excludesWe could create a "Spring Boot Session" starter that pulls in all technology specific JARs but we add Drop technology specific out-of-the-box supportWe could change the "Spring Session Starter" so that it only pulls in
The last option is probably my favorite so far as it seems the least confusing for users. I'm still not sure that it's as nice as the 1.5 behavior. Do you have any thoughts on the best way to proceed? |
Just to add some references to your comment @philwebb, the Initializr issue was reported in spring-io/initializr#455 while Spring Session starter(s) was proposed a while ago in #6082 (obviously the context of that proposal has changed now). The direction of Spring Session is shaping into somewhat similar direction to one with Spring Data and Spring Social. Not to repeat the background of the change (if it matters) I'll just point to spring-projects/spring-session#768 and this blog post. Regarding the multiple Session options on Initializr I'd like to point out one thing - with that option a user that (for example) uses a relational database but would like Redis backed sessions would pick |
The more I think about it, the more I wonder if the multiple options on start.spring.io might actually be the best option. I know that there are some reservations about adding more items, but it does actually make a lot of sense when you consider:
What are the downsides:
|
IMO, likening Spring Session to Spring Social or Spring Data doesn't really make sense. With Spring Social or Spring Data you are interacting directly with a store- or social network-specific API. With Spring Session you are interacting with a general purpose API that, almost as an implementation detail, can use different backing stores. Spring Session feels more like something like Spring Data JPA to me. You interact with a general purpose API that can use different RDBMSes for persistence. We don't have starters or checkboxes on start.spring.io for Data JPA with MySQL, Data JPA with Postgres, etc. I don't think we should have similar checkboxes for Spring Session either. Unfortunately, from a usability perspective at least, the new arrangement feels like a step backwards. |
There's certainly a lot to consider here. This is one of those unfortunate problems where in isolation everything makes sense, but when we try to pull it back together there are issues:
Once you decide that you want Spring Boot to be in control of data technologies however, things start to clash. The Redis driver pulled in by Spring Session might not be the one you want to use. What if you want to use I wonder if we can reach some middle ground where the We can then have a |
What Andy said. I would have preferred the start.spring.io topic to be brought in spring-io/initializr#455 but since we started here... From my experience, discoverability is far more important than expressiveness (and we want users to be focused). If you want to work with Redis, you add the redis checkbox. If you want to work with Spring Session, you add the spring session checkbox, etc. Sure you may realize later that the Spring Data Redis artifact is useless for your needs but you're 10.000 miles away of the decision that you took when you created the project. Besides, Worse than that, IMO, there is no way to get the functionality without those opinions. So if I want to use redis today, my only option is to add Regarding Spring Social, there is a "Spring Social Facebook" entry because there is no "Facebook" entry. If there was one, I'd have the exact same argument. We have a "Redis", "JDBC" and "MongoDB" entries so comparing Spring Session to Spring Social from that perspective doesn't make sense. Back to the actual change now, I most certainly have no opinion about splitting the code in several modules and having a release train. If the Spring Session team decides it's the right thing to do, that's fine by me. But, the new jar structure breaks the use case of 1.5 and that's a concern from our perspective. Ironically enough, those news jars are now far more easier to use if you are not using Spring Boot and much harder if you do (per my argument above about runtime choices taken by starters). Here is a proposal: what is wrong with having independent modules as you do now and build the same |
I can certainly go with the idea that if you want Redis you'll use it for everything. Same for MongoDB. Anything more complex should be annotated in Spring Session's reference docs. Final question, what should default be? If Op picks "Session" on start.spring.io and nothing else, what does it do? |
I like the JPA comparison. What does it do? |
Thank you for your input @wilkinsona @philwebb and @snicoll
This is not strictly true with Spring Data since it provides a common API for various data stores. For example, Spring Data Mongo, JPA, LDAP, Redis, Cassandra, etc all provide different implementations of Of course there are implementation specific APIs as well. However, there is a core API in I think it is important to note that there are implementation specific starters for Cloud Cluster (Redis, Hazelcast, and Etcd). Additionally, there are implementation specific starters for Discovery (Eureka, Zookeeper, and Consul). There are plenty of other artifacts in Initializr that only vary by their implementation. I really don't think the idea of having implementation specific starters for Spring Session is much different (if at all) than what is already present in our existing ecosystem.
The difference is Spring doesn't provide the JPA implementations of MySQL, Postgres, etc. Those are provided by third parties. Therefore, I do not see Spring Session as the same as your analogy. Spring Data does provide an implementation for JPA, Redis, etc. Similarly Spring Cloud provides discovery with Eureka and Zookeeper. Along the same lines, Spring Session provides implementations for JDBC, Redis, etc. Again, I don't see this much different than what Initializr is doing with other projects.
I'd like to understand why you find this a step backwards. Can you expand on this?
I think this makes a lot of sense. In fact, there is already a ticket in Spring Session to remove the Redis driver being included. See spring-projects/spring-session#802 It would probably be beneficial to take a look at all the transitive dependencies for the
However, I do not like the idea of an aggregate pom that combines each of the implementations. This makes it less clear what the user's intent is when they want Spring Session. More concretely, it means that we must continue to require the users provide
Unfortunately it isn't that simple. If a user has both MonogoDB and JDBC (or any other permutation) on the classpath, which Spring Session implementation should be used? We don't know and this is why we require
Right. I'd rather users decide what they want up front rather than being 10.000 mi away when they realize they made the wrong decision. Require the user make the right choice up front.
I think this is a valid point. We plan to address that in spring-projects/spring-session#802
Perhaps we can create a Session category for 1.x and 2.x. For Boot 2.0+ we would have
I think there are some valid concerns here, but this seems to be mostly with transitive dependencies. Here we agree. I think if we clean up the transitive dependencies of
I don't think there is the need for an uber jar. User ExperienceLet's talk about the user experience today vs what it could be with the current arrangement. It appears that we agree we need to sort out transitive dependencies for Spring Session 1.x
Spring Session 2.x
Note that if we have an Uber jar or aggregation pom we go back to the state things were in 1.x where we have extra setup from users and can get into invalid setups. |
I disagree. When you're using Spring Data, the code that you write is coupled to the datastore that you're using. It provides a familiar and similar API on top of those stores, but it is not an identical API across all of the stores. A developer still cares about
IMO, from a user's perspective, this isn't similar. A typical Spring Session user doesn't care about the different implementations of
No analogy is perfect, but I do think the one I used is closer than a Spring Social or Spring Data analogy. Another analogy could be Spring Framework's support for various view template technologies. All of the code is in spring-webmvc with optional dependencies on the template library. On start.spring.io you choose to build a web app and you choose a template library separately. That makes sense as you may be using the template library with MVC or in some other capacity. Just as you may be using Redis, Mongo, etc with Spring Session or in some other capacity.
This was really just a short way of saying that I agree with all of the cons that have already been listed. My main concern is the effect that the new split will have on start.spring.io. Spring Cloud already has too many checkboxes there (service discovery for example) and I don't want that broken window to influence what we do with Spring Session. Related, it'll also have a negative effect on Spring Boot's starters:
At this point, would it not be better if Spring Session just grouped everything together in a single jar? Then a user adds a Spring Session dependency and, if they're not already using a datastore, a dependency on that too. Our auto-configuration will then figure out which Spring Session repository type to use. Further configuration is only required if there's more than one Spring Session supported datastore available and we need to know which one to use. Lastly, I think it's worth noting that I haven't seen a single complaint about the current arrangement that we have for using Spring Session in Boot and on start.spring.io. From a Boot and start.spring.io perspective, the change appears to be trying to solve a problem that doesn't exist and is, I feel, making things worse in the process. |
That's not how users, and especially newcomers, are using start.spring.io in my experience. I actually meant the contrary: when they'll fine tune their projects later they may consider they don't need such and such dependencies and just provide more explicit ones. But that's 10.000 miles away from their need when they go to the site, that is getting started.
Unfortunately, I don't think so. Let's go forward a bit. The Redis driver is removed from
A third option would be for you to create a separate artifact in Spring Session that brings the implementation and the driver. That looks very familial to what Spring Boot is doing with the starter, isn't it?
Try to add JPA without any extra dependency and see what happens. Roughly 20% of all generated projects on start.spring.io use JPA and if you don't add a database you'll end up in an invalid state. Given the number of people complaining about this, I think we can safely conclude that this is working for our users.
What you describe a bug is a feature in my book. The store type has to be explicit. The fact you've split the implementations in separate jars doesn't shield you from having several
Andy summarized perfectly how I feel about this, +1 to that! |
Sorry, I forgot to reply to that. I think it's important to note that those fall in the category of things I wish we didn't do that way. And partly the reason why I insist we don't make the same mistake with Spring Session. Those are deprecated as well so I really don't think we can use that as an argument. |
Just to note the outcome of a call we had today on this subject. There's still not 100% agreement on the correct approach, but in order to move forwards this is the plan.
In addition several things will be enhanced in Spring Boot
Additional considerations:
|
The Spring Initializr change is available on the dev instance and here is the code snicoll/initializr@a989188 |
I thought this and #9625 were on hold due to unwillingness of Boot team to add an additional dependency management property for Spring Session MongoDB? BTW we are planning to have Spring Session BOM available soon. |
* gh-9552: Improve Spring Session MongoDB support naming
This commit could, largely, be reverted. The project now lives on separately. A 2.0.0.M1 release is available from repo.spring.io.
The text was updated successfully, but these errors were encountered: