-
-
Notifications
You must be signed in to change notification settings - Fork 353
Lift version of spring cloud 2025 m4 #1498
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
base: main
Are you sure you want to change the base?
Lift version of spring cloud 2025 m4 #1498
Conversation
|
Hey @MatejNedic, I suggest that the project follows the pattern the Spring team did for their migration, in order to support both Jackson v2 and v3 as they do. The pattern is, roughly:
Here's an example with DefaultKafkaHeaderMapper and JsonKafkaHeaderMapper At some decision points, they check which Jackson versions are in the classpath, such as in MessagingMessageConverter This is the PR from Spring Kafka's migration for reference: spring-projects/spring-kafka#3996 Let me know if that makes sense to you. |
|
Hey @tomazfernandes Sounds good to me! I have seen while moving to jackson 3 Deprecated on top of jackson 2 convrter classes. Can you make the changes required against this PR? Just check it out and push freely. I sadly won't have time till middle of next week. |
|
Unfortunately, I won't have the time either. I think it's fine for us to leave this migration for M2. I'll open an issue to track this, so the community can pitch in as well. |
|
Created the issue: #1511 Feel free to edit if you'd like. |
tomazfernandes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On hold until the work is aligned with the direction the team agreed on.
|
Lets go with M1 release with Jackson 2 support! |
📢 Type of change
📜 Description
@tomazfernandes everything regarding converters I have changed. I am not sure what is best way to tackle JsonMappers since JacksonJsonMessageConverter hides it. There is no way to set only to propagate one when using constructor.
I am tbh not sure what is best action here but it is a breaking change so please look and lets agree how to move. Be free to checkout PR and push directly :)
There are also few tests I have put to ignore till we settle the approach.
I tbh would go that users provide their own converters personally. JsonMapper default one already is registering few modules so yea.
I have to check cloudwatch integration currently it is failing I will check it tomorrow evening.
Also I have to rework docs since a autoconfiguration will prolly change!
💡 Motivation and Context
💚 How did you test it?
📝 Checklist
🔮 Next steps