You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2020/10/15 19:59:16 UTC
[GitHub] [incubator-daffodil] stevedlawrence opened a new pull request #439: Allow CLI unparse to unparse from a stream of data
stevedlawrence opened a new pull request #439:
URL: https://github.com/apache/incubator-daffodil/pull/439
The CLI currently reads in the entire infoset into memory and unparses
from that. This means that the CLI cannot be be used to unparse very
large infosets due to memory limitations.
In some cases we do want to read the infoset into memory, such as with
the performance command to avoid IO latency, or with the --stream
option, which must scan the input looking for the end of each stremamed
infoset. We also must do this if we are unparsing from something like a
Scala XML Node, since we must read the entire infoset to convert it to a
Node.
But in cases where possible, we should not read the entire infoset into
memory, and instead just treat is as an InputStream and let the
InfosetInputter read in the infoset as needed.
This changes the infosetDataToInputterData function to accept either an
Array[Byte] or an InputStream and return an InputStream where possible.
The getInfosetInputter function then uses the appropriate InputStream
for the InfosetInputter, thus avoiding reading the entire infoset into
memory in some cases.
DAFFODIL-2412
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence merged pull request #439: Allow CLI unparse to unparse from a stream of data
Posted by GitBox <gi...@apache.org>.
stevedlawrence merged pull request #439:
URL: https://github.com/apache/incubator-daffodil/pull/439
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on pull request #439: Allow CLI unparse to unparse from a stream of data
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #439:
URL: https://github.com/apache/incubator-daffodil/pull/439#issuecomment-710082987
Yeah, I think you're reading the codecov right. It was correct that parts of this diff weren't covered, but how it showed that, as you say, doesn't feel right. If I run the commands to generate a coverage report locally, it feels a little more correct, but still not quite right. I think the challenge is just that Scala doesn't a bunch of stuff behind the scense, and the coverge tools just can't make an accurate represetnation of what's going on.
But it was correct that we didn't have CLI tests for different infoset types, and so no actual coverage for some of these changes I just made, and I think it was worth adding just to make sure we don't have any regressions in the future. The way this infosetDataToInputterData and getInfosetInputter is sort of fragile since they retun/accept AnyRef, so it's important that they have a little coverage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on pull request #439: Allow CLI unparse to unparse from a stream of data
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #439:
URL: https://github.com/apache/incubator-daffodil/pull/439#issuecomment-710055503
Added some CLI tests should add more coverage to Main.scala, including this new code. I hate seeing that little red :x:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org