-
Notifications
You must be signed in to change notification settings - Fork 232
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
Added maximum string length constraint. (#449) #452
base: master
Are you sure you want to change the base?
Conversation
@@ -78,6 +79,7 @@ public class CucumberReportPublisher extends Recorder implements SimpleBuildStep | |||
private boolean undefinedAsNotFailingStatus; | |||
|
|||
private int trendsLimit; | |||
private int maxStringLengthConstraint = 20_000_000; |
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.
nice,
would be perfect having reference to the code from that value was taken
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.
Hi @damianszczepanik, here is the refence (the default value used by jackson-core, line 46):
https://github.com/FasterXML/jackson-core/pull/1019/files#diff-41ffdb6337d4e68c7ae85301ca9f13fb7fcfe00a3244e3b70a604bf49fa12efe
@@ -559,6 +570,8 @@ private void generateReport(Run<?, ?> build, FilePath workspace, TaskListener li | |||
|
|||
setFailingStatuses(configuration); | |||
|
|||
StreamReadConstraints.overrideDefaultStreamReadConstraints(StreamReadConstraints.builder().maxStringLength(maxStringLengthConstraint).build()); |
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.
that should be implemented in core component https://github.com/damianszczepanik/cucumber-reporting
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 it.
@lucascz37 ping |
Are you @lucascz37 going to work on this ? |
Hi @damianszczepanik sorry for the delay, I was on vacation, I will work on this in the evening today |
<f:entry | ||
title="${%maxStringLengthConstraint.title}" | ||
field="maxStringLengthConstraint"> | ||
<f:textbox default="20000000"/> |
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.
Perhaps we should consider bumping it a little by default. Maybe 25M? This would cover the needs of all the people at #449.
What do you think, @damianszczepanik?
In such case, maybe making it configurable isn't even required (unless someone makes a case where a higher constraint is needed).
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.
@damianszczepanik , would you consider this simple fix from @felipecrs for #449?
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.
@damianszczepanik, if you agree, I can send a PR for it.
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.
IMO solution should be implemented first in https://github.com/damianszczepanik/cucumber-reporting
Hi is there any updates on this PR? |
Adds maxStringLengthConstraint field to personalize the readStreamConstraints of JacksonCore. By default the maxStringLength of the StreamReadConstraint is 20_000_000, from what I understood if the json files surpass that size it will throw the error described in issue #449 .
Testing done
I executed three builds with .json files from my tests.
Relevant links:
https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.15.2
FasterXML/jackson-core#1014
Tests screenshots:
Test 1:
Test 2:
Test 3:
Submitter checklist