-
-
Notifications
You must be signed in to change notification settings - Fork 95
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
FE: Fix Default Consumer's Lag Value #686
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi K-Diger! 👋
Welcome, and thank you for opening your first PR in the repo!
Please wait for triaging by our maintainers.
Please take a look at our contributing guide.
|
Further user feedback is requested. Please reply within 7 days or we might close the issue. |
Since the server responds with exactly 0, I think this is different from the situation where it displays 'N/A' for the issue you tagged.
The value displayed in the JSON response above is the same as the photo I attached. In conclusion, the tagged issue appears to be a proposal to indicate N/A when data cannot be loaded. What I am suggesting is to not represent 0 as 'N/A'. Also In the URL above, the receiving consumer LAG will receive something labeling 0 if it receives 0 from the server.
|
Thanks for the additional feedback! We'll get back to your issue soon. |
In this case, I don't think your changes make sense. |
I've revised the code again based on your feedback. This time, the modified code displays the response as 0 only when it is 0. I modified it to display 'N/A' when the value cannot actually be retrieved, such as null. |
Thanks for your reply. We have made a commit that reflects your feedback,
so please check.
2024년 12월 19일 (목) 오전 12:56, Roman Zabaluev ***@***.***>님이 작성:
… Since the server responds with exactly 0, I think this is different from
the situation where it displays 'N/A' for the issue you tagged.
In this case, I don't think your changes make sense.
The lag is 0 in your response ("consumerLag": 0), which, under current
implementation, (return args.getValue() || 'N/A';) will be displayed as
is. args.getValue() would be false only in case of undefined, not 0. Am I
missing something here?
—
Reply to this email directly, view it on GitHub
<#686 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOOCHT4TOJS6HQDIHDTA6BD2GGLMXAVCNFSM6AAAAABSYF2N3KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJRGY4TONZWGY>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
Co-authored-by: Roman Zabaluev <gpg@haarolean.dev>
…headless in /api (kafbat#700) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Roman Zabaluev <gpg@haarolean.dev>
88caa2b
to
37ca435
Compare
I've added my comments on what you said and a commit for additional test
code.
We would appreciate it if you could give us feedback after checking.
|
close #675
The default value of Consumer Lag displayed when searching the Consumer List has been modified.
The existing displayed value is 'N/A', which is different from the response value of '0' that the server responds to.
We believe that 'N/A' may mean a state of disuse, so we write it as 0 to make it clear.
What changes did you make? (Give an overview)
Is there anything you'd like reviewers to focus on?
How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)
What changes did you make? (Give an overview)
Is there anything you'd like reviewers to focus on?
How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)
Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)
Check out Contributing and Code of Conduct
A picture of a cute animal (not mandatory but encouraged)