NIFI-4142: Refactored Record Reader/Writer to allow for reading/writi…#2015
NIFI-4142: Refactored Record Reader/Writer to allow for reading/writi…#2015markap14 wants to merge 1 commit intoapache:masterfrom
Conversation
|
|
||
| public class TypeMismatchException extends RuntimeException { |
There was a problem hiding this comment.
i dont think it is ok to change this exeception class name at this juncture and even if it is questionable ok the juice is probably not worth the squeeze. TypeMismatch and SchemaValidation are pretty much the same thing
There was a problem hiding this comment.
I don't agree that they are pretty much the same thing. TypeMismatchException is very specific. SchemaValidationException can be much more broad. For instance, if a required field is missing, that is not a Type Mismatch, but it is a Schema Validation.
| * @throws SchemaValidationException if a Record contains a field that violates the schema and cannot be coerced into the appropriate | ||
| * field type and schema enforcement is enabled | ||
| */ | ||
| Record nextRecord(boolean enforceSchema) throws IOException, MalformedRecordException; |
There was a problem hiding this comment.
the schema had always been enforced arguably just with sense of leniency. I think this method parameter should be 'strictSchemaEnforcement' or 'enforceStrictSchema'.
There was a problem hiding this comment.
I think I actually want to just separate the concept out into two different variables here: boolean coerceTypes, boolean dropUnknownRecords. That way it is very explicit what is happening, and I don't think that 'strict' vs. 'lenient' really conveys those two semantics as well as I'd like.
| * @throws SchemaValidationException if a Record contains a field that violates the schema and cannot be coerced into the appropriate field type. | ||
| */ | ||
| Record nextRecord() throws IOException, MalformedRecordException; | ||
| default Record nextRecord() throws IOException, MalformedRecordException { |
There was a problem hiding this comment.
we should indicate whether the scheme enforcement strictness is 'lenient' or 'strict'.
|
@joewitt I've pushed a new commit that I believe better clarifies how schemas are treated in terms of strictness vs. leniency by providing two arguments instead of 'enforceSchema': 'coerceTypes' and 'dropUnknownFields' |
|
@markap14 can you please rebase and resolve conflict and please go ahead and squash. |
…ng "raw records". Implemented ValidateRecord. Updated Record Reader to take two parameters for nextRecord: (boolean coerceTypes) and (boolean dropUnknownFields)
|
@markap14 looks like the option says "Used Reader's Schema". I'm assuming that is a typo and it should say "Use Reader's Schema". But that is not in this PR it appears. Did that sneak in elsewhere? |
| "The schema to validate the data against is determined by looking at the 'Schema Name' Property and looking up the schema in the configured Schema Registry"); | ||
| static final AllowableValue SCHEMA_TEXT_PROPERTY = new AllowableValue("schema-text-property", "Use Schema Text Property", | ||
| "The schema to validate the data against is determined by looking at the 'Schema Text' Property and parsing the schema as an Avro schema"); | ||
| static final AllowableValue READER_SCHEMA = new AllowableValue("reader-schema", "Used Reader's Schema", |
There was a problem hiding this comment.
Ah here it is. "Used Reader's Schema" should be "Use Reader Schema".
|
went ahead and fixed the typo and squashed locally. Ran a test flow that created csv of various conformity to a simple spec. Ran it through various configratuions of the validator and it behaved as desired and expected in each case. When data was marked invalid was able to see precisely why through the provenance events which was great. Was able to see coercion/non-coercion behavior and all checks out. +1 merging to master |
2 participants
…ng "raw records". Implemented ValidateRecord.
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.