Ignore features in FeatureRow if it's not requested in import spec #101
Conversation
|
/lgtm thanks for adding the tests :) |
|
We shouldn't completely ignore them in all these places. We need errors so that we know what's happening, we don't want to silently discard them. So what you seem to want is that instead of throwing the whole row into the errors pile, you want to accept as many features as possible and just throw errors for these features. The only place that needs to do that is the ConvertTypesDoFn. I suggest we should instead emit the row without these suspect features, while also emit an error row (to the errors tag) with just the suspect feature. If these features proceed past ConvertTypesDoFn, it's a bug, not something we should ignore. |
|
/hold |
|
The issue is that they're not necessarily errors. We want to be able to selectively ingest features from a given feature row in the streaming case - similar to how we do it in batch. |
|
I agree, I think we can do it only on ConvertTypesDoFn since it's the most upstream transform.
Although, I feel emitting error row might not be a good idea since the stream might have a lot of features we are not interested at and it will just flood the error logs. |
|
/approve |
|
/hold cancel |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tims, zhilingc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
Fix #99