LB-171 / LB-172: Fix bugs in influx-writer #197
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
|
DUMP_JSON_WITH_ERRORS is true while we are in beta. |
|
That was causing LB-172 then for sure. |
Not doing this caused a bug where if a field had a list, it wouldn't get converted and would cause a parse error in influx when we submitted it.
`data` is a list of Listen objects. When we try to json.dump that list, it raises an error and causes the influx-writer to die. This is what causes LB-172 too, afaik.
Changed additional_info.json to add a new field that contains a list to test for the bug causing LB-171.
paramsingh
force-pushed
the
influx-writer/bugs
branch
from
889d4ab
to
344044b
Compare
mayhem
approved these changes
Jun 19, 2017
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.
LB-171: marc2k3's listens weren't being inserted because they contained a list in their additional_info field which led to parse errors (this seems to be a bug in influx because other non-string entities like uuids were inserted correctly). Converting all new fields in additional info to strings explicitly now fixes that. Also added tests for listens like this.
LB-172: While doing this, I found a problem that might be causing LB-172. Influx-writer was dying because the debug message tried to serialize an array of Listen objects. However this would only trouble us if
DUMP_JSON_WITH_ERRORSwasTruein production, and I'm not sure that it was.