Skip to content
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

Draft: Config support #521

Closed
wants to merge 11 commits into from
Closed

Draft: Config support #521

wants to merge 11 commits into from

Conversation

pjfanning
Copy link
Member

No description provided.

@cowtowncoder
Copy link
Member

In general, builder-style configuration is being added to most Jackson modules so that might make sense here too, even if providing full Config objects?

@pjfanning
Copy link
Member Author

@cowtowncoder my preference is still to add a jackson-databind DeserializationFeature and to uptake it in jackson-module-scala. This is just an alternative approach if we decide to keep the solution entirely in jackson-module-scala.

The typesafe config is a commonly used way to configure scala libs and helps to avoid adding new APIs for setting config.

@cowtowncoder
Copy link
Member

@pjfanning I know typesafe config is widely used it's just that my experience suggests that usually such libraries/frameworks work better for main level applications/services/clients and can be less optimal (when doing automatic classpath scanning etc) for embedded components, which is a common use case for Jackson.
Challenges usually come from cases where Jackson is used by multiple unrelated subsystems (or app and one of its dependencies). Because of this I prefer a little bit of explicitness -- even if only requiring call like "just load typesafe config (from specific section)".

But that is just a suggestion; if you think configuration as specified makes sense that is fine.

Ditto wrt Jackson features.

@pjfanning
Copy link
Member Author

pjfanning commented Jun 24, 2021

@cowtowncoder this config solution is not a solution I like but adding the current jackson-module-scala code was written in a way that makes configuring it using a Module Builder very hard without breaking binary compatibility. See https://github.com/FasterXML/jackson-module-scala/blob/master/src/main/scala/com/fasterxml/jackson/module/scala/DefaultScalaModule.scala and the way the submodules are traits, meaning I can't pass stateful class instances into them.

A Module Builder would be a nice feature for jackson-module-scala v3 but I think it would be easy in v2. And so far, we have just a small number of v2 users asking for 1 config option.

My preferred option for jackson v2 is FasterXML/jackson-databind#3181 or a variant of it - that would would allow me to get the config from the existing jackson-databind classes.

@cowtowncoder
Copy link
Member

@pjfanning Ok I don't have any big problems with another feature to add and it is a fair point that there's just this one request for now.
I was guessing that "fake" Builder which other modules use might work, but it sounds like the structuring of Scala module (much more based on sort of loose composition if I understand it?) makes that impractical for 2.x.
Thank you for explaining the situation.

@pjfanning
Copy link
Member Author

Using a MapperFeature instead

@pjfanning pjfanning closed this Jul 2, 2021
@pjfanning pjfanning deleted the config-support branch July 2, 2021 08:39
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.

None yet

2 participants