stevedlawrence commented on code in PR #1322:
URL: https://github.com/apache/daffodil/pull/1322#discussion_r1777768368
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/SDE.scala:
##########
@@ -262,14 +262,26 @@ trait ImplementsThrowsOrSavesSDE extends
ImplementsThrowsSDE with SavesErrorsAnd
val suppress = lssdw.contains(warnID) || lssdw.contains(WarnID.All) ||
tssdw.contains(warnID) || tssdw.contains(WarnID.All)
if (!suppress) {
- val sdw = new SchemaDefinitionWarning(
- warnID,
- Some(schemaFileLocation),
- NoAnnotationContext,
- fmt,
- args: _*
- )
- warn(sdw)
+ if (tunable.escalateWarningsToErrors) {
+ val msg = "warnings escalated to errors: " + fmt
+ val sde = new SchemaDefinitionError(
+ Some(schemaFileLocation),
+ NoAnnotationContext,
+ msg,
+ args: _*
+ )
+ error(sde)
Review Comment:
Should this be `toss(sde)`? I think error will just record it but wont'
cause immediate failure and could continue.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/SDE.scala:
##########
@@ -262,14 +262,26 @@ trait ImplementsThrowsOrSavesSDE extends
ImplementsThrowsSDE with SavesErrorsAnd
val suppress = lssdw.contains(warnID) || lssdw.contains(WarnID.All) ||
tssdw.contains(warnID) || tssdw.contains(WarnID.All)
if (!suppress) {
- val sdw = new SchemaDefinitionWarning(
- warnID,
- Some(schemaFileLocation),
- NoAnnotationContext,
- fmt,
- args: _*
- )
- warn(sdw)
+ if (tunable.escalateWarningsToErrors) {
+ val msg = "warnings escalated to errors: " + fmt
+ val sde = new SchemaDefinitionError(
Review Comment:
Should we include the warnID so that a user could know what to suppress if
they want to suppress a particular warning?
Is there any value in in a new type of SDE like this:
```scala
val sdw = ...
val sde = new SchemaDefinitionErrorFromWarning(sdw)
```
Note a fan of the name, but then the logic for displaying the warn id is
exactly the same regardless of if it's escalated or not. And if we ever change
how warnings work we don't really have to change this.
Not sure if adding an extra SDE class that wraps an SDW gets messy though...
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/SDE.scala:
##########
@@ -262,14 +262,26 @@ trait ImplementsThrowsOrSavesSDE extends
ImplementsThrowsSDE with SavesErrorsAnd
val suppress = lssdw.contains(warnID) || lssdw.contains(WarnID.All) ||
tssdw.contains(warnID) || tssdw.contains(WarnID.All)
if (!suppress) {
- val sdw = new SchemaDefinitionWarning(
- warnID,
- Some(schemaFileLocation),
- NoAnnotationContext,
- fmt,
- args: _*
- )
- warn(sdw)
+ if (tunable.escalateWarningsToErrors) {
+ val msg = "warnings escalated to errors: " + fmt
+ val sde = new SchemaDefinitionError(
+ Some(schemaFileLocation),
+ NoAnnotationContext,
+ msg,
+ args: _*
+ )
+ error(sde)
+ } else {
+
+ val sdw = new SchemaDefinitionWarning(
+ warnID,
+ Some(schemaFileLocation),
+ NoAnnotationContext,
+ fmt,
+ args: _*
+ )
+ warn(sdw)
+ }
}
Review Comment:
I think we need a similar check in `def SDW` in ProcessorStateBases.scala
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
