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]


Reply via email to