Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use encodingErrorPolicy replace, even when set to error #1129

Conversation

stevedlawrence
Copy link
Member

If encodingErrorPolicy="error" we print a warning letting the user know it is not supported and that we will use "replace" instead. This is so that we can support schemas written for IBM DFDL (which only supports "error") and have the exact same behavior as IBM DFDL except for when data has encoding errors, which is usually rare.

However, even though we warn that we will use "replace", we do not actually do so, which leads to an assertion exception if there are encoding errors. This fixes that so our behavior matches the warning message and we always use "replace".

DAFFODIL-2861

Copy link
Contributor

@tuxji tuxji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

TermEncodingMixin.scala comment looks good.

Copy link
Contributor

@mbeckerle mbeckerle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
One minor suggestion

If encodingErrorPolicy="error" we print a warning letting the user know
it is not supported and that we will use "replace" instead. This is so
that we can support schemas written for IBM DFDL (which only supports
"error") and have the exact same behavior as IBM DFDL except for when
data has encoding errors, which is usually rare.

However, even though we warn that we will use "replace", we do not
actually do so, which leads to an assertion exception if there are
encoding errors. This fixes that so our behavior matches the warning
message and we always use "replace".

DAFFODIL-2861
@stevedlawrence stevedlawrence force-pushed the daffodil-2861-encodingErrorPolicy-error branch from 0612bfc to 2fe944a Compare December 12, 2023 13:10
@stevedlawrence stevedlawrence merged commit 37d6746 into apache:main Dec 12, 2023
10 checks passed
@stevedlawrence stevedlawrence deleted the daffodil-2861-encodingErrorPolicy-error branch December 12, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants