-
Notifications
You must be signed in to change notification settings - Fork 12
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
Disable validation of field lengths when parsing event #127
Comments
istreeter
added a commit
that referenced
this issue
Mar 13, 2023
istreeter
added a commit
that referenced
this issue
Mar 13, 2023
istreeter
added a commit
that referenced
this issue
Mar 13, 2023
istreeter
added a commit
that referenced
this issue
Mar 13, 2023
pondzix
pushed a commit
that referenced
this issue
Nov 29, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is about reversing the change we made in #115
In sdk version 3.0.0 we changed the Event parsers so that a parsing would fail if the event's fields exceeded the maximum lengths allowed by the atomic event schema. The change was important at the time, because we could not rely on Enrich to validate the atomic field lengths -- that functionality was only added in Enrich 3.0.0.
Meanwhile, it was important for Snowplow's loaders to have guarantees that the events it received would not exceed the max lengths on the warehouse tables.
But now, 1 year later, it does not seem like the right design for the analytics sdk to validate these fields. It is better for validation to happen in one place only, and that should be Enrich. In almost all scenarios, we can trust that events emitted by Enrich conform to the atomic field lengths. The only exceptions are if enrich is running with the
featureFlags.acceptInvalid
config option, or if the analytics SDK is processing historic data produced by an older version of Enrich.We should keep the validation code in the analytics sdk, because it is still needed by some loaders under some circumstances. But validation should be off by default.
The text was updated successfully, but these errors were encountered: