stevedlawrence commented on code in PR #1515:
URL: https://github.com/apache/daffodil/pull/1515#discussion_r2251172441
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -934,18 +934,12 @@ abstract class TestCase(testCaseXML: NodeSeq, val parent:
DFDLTestSuite) {
_.nBits
}
- val useSerializedProcessor =
- if (validation == "on") false
- else if (defaultValidation == "on") false
- else if (optExpectedWarnings.isDefined) false
- else true
-
val rootNamespaceString = getRootNamespaceString()
val compileResult: TDML.CompileResult = parent.getCompileResult(
impl,
suppliedSchema,
- useSerializedProcessor,
+ useSerializedProcessor = true,
Review Comment:
Can we completely remove all occurrences of useSerailizedProcessor? This is
now always true and we always use a serialized processor, so I don't think it
should be needed anymore.
##########
daffodil-tdml-processor/src/main/scala/org/apache/daffodil/processor/tdml/DaffodilTDMLDFDLProcessor.scala:
##########
@@ -113,6 +113,7 @@ final class TDMLDFDLProcessorFactory private (
val diags = p.getDiagnostics
Left(diags)
} else {
+ val diags = p.getDiagnostics
Review Comment:
`val diags = p.getDiagnostics` is now duplicated as the first thing in each
if-block branch. Can we move this outside of the if block to remove the
duplication?
##########
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/processors/DataProcessor.scala:
##########
@@ -155,8 +116,7 @@ class DataProcessor(
* @return the serializable object
*/
@throws(classOf[java.io.ObjectStreamException])
- private def writeReplace(): Object =
- new SerializableDataProcessor(ssrd, tunables, variableMap.copy())
+ private def writeReplace(): Object = new DataProcessor(ssrd, tunables,
variableMap.copy())
Review Comment:
I wonder if we should completely remove this writeReplace?
The idea being that if someone wants to directly serialize the DataProcessor
(which can happen in frameworks like Spark) then they can do so, and they will
get the exact DataProcessor that was created, including things this
writeReplace excludes like validator, debugger, and diagnostics. The one issue
is that there is the potential that not all of those are serializable (e.g. the
Xerces validator is not currently serializable), but we should let users deal
with that instead of silently removing things.
And in cases people will use `save`/`reload`, which will exclude the
validator, debugger, and diagnostics since that's usually the expected
behavior. And we should ensure the save() API documents that any changes due to
withValidator, withDebugger, and any compile diagnostics are not saved.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
