Remove null values from events #82
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asatwal
force-pushed
the
remove-null-values-from-events
branch
from
a059968 to
29a98c2
Compare
duncanjbrown
approved these changes
Jul 11, 2023
Gemfile
Outdated
| @@ -6,6 +6,21 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } | |||
| # Specify your gem's dependencies in dfe-analytics.gemspec | |||
| gemspec | |||
|
|
|||
| group :development do | |||
There was a problem hiding this comment.
🤷 If this is what rubocop wants!
There was a problem hiding this comment.
This doesn't work so I had to enforce the gemspec style. This required rubocop updates and subsequent updates of all the Gemfiles.
lib/dfe/analytics/event.rb
Outdated
| @@ -115,10 +110,14 @@ def convert_value_to_json(value) | |||
|
|
|||
| def hash_to_kv_pairs(hash) | |||
| hash.map do |(key, values)| | |||
| values_as_json = Array.wrap(values).map do |value| | |||
| convert_value_to_json(value) | |||
| if Array.wrap(values).count(&:nil?).positive? | |||
There was a problem hiding this comment.
Suggested change
| if Array.wrap(values).count(&:nil?).positive? | |
| if Array.wrap(values).any?(&:nil?) |
There was a problem hiding this comment.
Yes. That's better !
lib/dfe/analytics/event.rb
Outdated
| values_as_json = Array.wrap(values).map do |value| | ||
| convert_value_to_json(value) | ||
| if Array.wrap(values).count(&:nil?).positive? | ||
| message = "event has missing values in data - event: #{@event_hash} key: #{key} values: #{values}" |
There was a problem hiding this comment.
Suggested change
| message = "event has missing values in data - event: #{@event_hash} key: #{key} values: #{values}" | |
| message = "an array field contains nulls - event: #{@event_hash} key: #{key} values: #{values}" |
asatwal
force-pushed
the
remove-null-values-from-events
branch
2 times, most recently
from
974239d to
d673ca3
Compare
asatwal
force-pushed
the
remove-null-values-from-events
branch
from
d673ca3 to
c48cdd9
Compare
duncanjbrown
approved these changes
Jul 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trello-1253
In TV a number of events are not being sent to BQ from the vacancies table. When we attempted a backfill, there were a lot of errors in the sidekiq dead queues.
After adding more diagnosis to the error logs (See PR), we were able to find the bug.
In the values field of the event data, some value array fields have
nilvalues.For example these entries for the vacancies table are causing API errors:
The following changes have been made by this PR:
nilvalues and write a warning message to the logs