stevedlawrence commented on a change in pull request #520:
URL: https://github.com/apache/daffodil/pull/520#discussion_r608633044
##########
File path: daffodil-japi/src/main/scala/org/apache/daffodil/japi/Daffodil.scala
##########
@@ -866,6 +867,12 @@ class ParseResult private[japi] (pr: SParseResult,
deprecatedOutput: Maybe[JDOMI
* @return true if any validation errors occurred, false otherwise.
*/
def isValidationError(): Boolean = pr.isValidationError
+
+ /**
+ * Optional result of the infoset validation.
+ * @return optional ValidationResult
+ */
+ def validationResult(): Option[ValidationResult] = pr.validationResult()
Review comment:
> Question is/was, does it scale? I say not beyond a low number of
shortish parameters.
Fair, but I don't think the current solution scales all that well either--it
only supports a single option and a single output stream.
As an alternative to what I suggested earlier, maybe we do something where
the .conf file (not sure what the syntax is for specifying that, can' find any
test) can/should be used for configurations that are static (e.g. an encoding
option, enable/disable features), but additional options can be provided to
override those, or when more dynamic options are needed like in this case. We
could also use an alternative syntax that is maybe less ugly, for example:
```
--validate foo.sh --validate-option config=foo.conf --validate-option
output=details.svrl
```
We have very similar syntax for defining external variables, e.g.
``-Dvariable1=value1 -Dvariable2=value2``
So ``--validate`` can be provided once to specify the mode/validator plugin,
and ``--validate-option`` is a key-value pair that can be specified multiple
times for different options. One option could provide a config file to
configure the validator, other options could override those when needed or to
provide more dynamic options.
An added benefit (depending on how you view it?) is that now validators are
responsible for supporting/parsing the config option, and so they can use
whatever configuration format makes sense for them. Maybe some validators
already have a well defined and popular config file format that is different
than what we currently support. If these are truely pluggable, we might not
want to force a specific format.
> Conceptually there are two streams of data that come out of validators
While I agree that's probably the case, I'm not sure a byte array is always
the most natural way to get the second thing that could come out of a
validator. I'd argue when using the API, we never want a byte array--we usually
want specific underlying data structures with casting probably being the best
way to get to them. And when not using the API, then I'd rather assume the
validator knows best what information should be output, where, and what the
most efficient way to do it, and using some config stuff to tweak it.
All that said, if you're not sold, I won't stand in the way of this change.
This definitey provides a useful feature that we do need, and it is well done.
And if we do eventually decide we need something different is this is run
through its paces, we can always go through the normal deprecation process. For
example, it would be pretty trivial to to change ``--validate-output=foo`` to
``--validate-option output=foo`` behind the scenes.
+1 :+1:
--
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:
[email protected]