Allow override of StreamReadContraints default with overrideDefaultStreamReadConstraints() by pjfanning · Pull Request #1019 · FasterXML/jackson-core (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation7 Commits7 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
if this approach is acceptable, I can extend this PR with some tests
As per my other note, I don't want to make API changes in 2.15. Will release 2.15.1 soon; this PR may be left as fallback in case there really is enough push back to require addition (i.e. I can still consider change, 2.15.2 or 2.15.1 makes little difference semantically).
@cowtowncoder I am just one person, but consider this my pushback: it's extremely inconvenient for legacy projects that are already handling strings* >5MB or greater in production to have no mechanism to configure the default globally. You can see another example in Github where Palantir have had to jump through hoops and use Reflection to override the default in their codebase.
Originally I wrote "documents" but I meant "strings". It just so happens that in our largest documents, string size is equivalent to document size
@darrenoc3 Documents can be as big as necessary, 5 MB is for String values, not documents (currently no limit although we'll likely introduce mechanism for enabling one).
I am also bit surprised about P as a company since I thought we had feedback from them during pre-release phase suggesting 5 meg limit was actually fine for them in particular (I may have mistaken company with another one tho).
But fwtw as per #1014 limit will be bit higher (20 MB) in 2.15.1, to be released today.
@cowtowncoder I think this is the Palantir change - palantir/conjure-java-runtime#2601 - they are using the 2.15.0 API but setting a 50Mb per string limit, just using Reflection because they want to support jackson 2.14 too.
They seem to have control over the ObjectMappers that get created. Many users do not - they get them created via 3rd party libs - and there is no rush by 3rd party lib maintainers to allow users to configure the ObjectMappers that the 3rd party libs create.
I am coming around to thinking this would make sense for 2.15.2. I'll think about it bit more but will probably merge it.
@pjfanning Is this ready to be merged? Only asking as it's still in Draft stage.
[DRAFT] allow override of StreamReadContraints default allow override of StreamReadContraints default
allow override of StreamReadContraints default Allow override of StreamReadContraints default
Allow override of Allow override of StreamReadContraints defaultStreamReadContraints default with overrideDefaultStreamReadConstraints()