Filter out double values with NaN #3
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
c33howard
reviewed
May 31, 2021
| @@ -97,6 +97,10 @@ module.exports = function(app) { | |||
| Time: `${point.timestamp}` | |||
| }; | |||
| }); | |||
|
|
|||
| // filter out NaN double values as they cause Timestream to reject the batch | |||
| records = records.filter((r) => (r.MeasureValueType != 'DOUBLE' || !Number.isNaN(Number(r.MeasureValue)))); | |||
There was a problem hiding this comment.
I'm curious why you decided to do this after the conversion to timestream's request format, rather than before. I can see pros and cons both ways, so curious as to your thought process.
My care factor is much lower for the rest of the comments on this single line change. :-)
I'm already using lodash, would _.reject() read better, since you've got a couple negations? https://lodash.com/docs/4.17.15#reject
For good or ill, I've also gone with the more verbose function() style, rather than the => style. Mind switching to blend in?
Also, my javascript-fu is weak. I'm a bit surprised it's legal to redefine records as it's const. Purely for my education, how should I be thinking about this?
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.
I have seen issues where in certain cases my GPS coordinates where NaN - which causes Timestream to reject the entire batch of values.
Filtering is done after initial filtering in line 60 to allow for type to be determined and only filter out DOUBLE values that are NaN.