Fixing NaN in coordinator UI on load#23285
Merged
yhwang merged 1 commit intoprestodb:masterfrom Jul 25, 2024
Merged
Conversation
anilsomisetty
force-pushed
the
issue_23186
branch
from
2d2ad67 to
404a315
Compare
Contributor
Author
|
Hey @elharo , could you please review this PR |
elharo
reviewed
Jul 24, 2024
Contributor
elharo
left a comment
elharo
left a comment
There was a problem hiding this comment.
Now that I look at this "Unknown" doesn''t seem quite right since there's no value yet. It's not that there is a value but it's unknown. There isn't a value. What if we just used the empty string here instead?
anilsomisetty
force-pushed
the
issue_23186
branch
from
404a315 to
9ddb2bc
Compare
Contributor
Author
|
yeah empty string seems right 
|
Member
|
LGTM with one minor comment: I believe those are statistics. Shouldn't those undefined values be |
yhwang
approved these changes
Jul 24, 2024
elharo
approved these changes
Jul 24, 2024
Contributor
|
Empty string seems correct to me. 0 and undefined/unknown are not the same |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
Currently on loading coordinator UI it shows NaN for some values, this change will check for NaN and would return Unknown in it's place.
Motivation and Context
Fixing issue #23186
Impact
After this change NaN would be shown as Unkown initially on load
Release Notes