-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: fixProducerAPI #6422
feat: fixProducerAPI #6422
Conversation
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
👍 Looks good to me! Reviewed everything up to 55a4056 in 54 seconds
More details
- Looked at
83
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. pkg/query-service/app/http_handler.go:3224
- Draft comment:
EnsureserviceName
andtopicName
are not nil before dereferencing. Add a nil check to prevent potential runtime panic. - Reason this comment was not posted:
Comment did not seem useful.
2. pkg/query-service/app/http_handler.go:3262
- Draft comment:
Ensuretopic
andserviceName
are not nil before dereferencing. Add a nil check to prevent potential runtime panic. - Reason this comment was not posted:
Marked as duplicate.
3. pkg/query-service/app/integrations/messagingQueues/kafka/translator.go:287
- Draft comment:
Ensure that changingPanelType
fromPanelTypeTable
toPanelTypeList
is consistent with the rest of the codebase and intended behavior. - Reason this comment was not posted:
Comment did not seem useful.
4. pkg/query-service/app/integrations/messagingQueues/kafka/translator.go:284
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_zr6j7WrEXROmwEAU
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
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.
👍 Looks good to me! Incremental review on bf74ac7 in 34 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. pkg/query-service/app/integrations/messagingQueues/kafka/translator.go:367
- Draft comment:
The condition still includesproducer-throughput-overview-latency
, which contradicts the PR description. Please remove it if it's no longer needed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is incorrect because the condition no longer includesproducer-throughput-overview-latency
after the change. The comment seems to be based on outdated information or a misunderstanding of the diff. Therefore, the comment should be deleted as it does not reflect the current state of the code.
I might be missing some context from the PR description, but based on the diff alone, the comment is incorrect. The condition was changed to removeproducer-throughput-overview-latency
, so the comment is not applicable.
Even if the PR description mentionedproducer-throughput-overview-latency
, the code change clearly shows it was removed, making the comment irrelevant.
Delete the comment because it is incorrect; the condition no longer includesproducer-throughput-overview-latency
.
2. pkg/query-service/app/integrations/messagingQueues/kafka/translator.go:367
- Draft comment:
The condition should includeproducer-throughput-overview-latency
to align with the PR description changes. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_laMnwmlnDyS4Jhif
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
This reverts commit d1503f1.
This reverts commit d1503f1.
Summary
Related Issues / PR's
https://github.com/SigNoz/engineering-pod/issues/2002
Screenshots
NA
Affected Areas and Manually Tested Areas
Important
Switch
getProducerThroughputOverview
to useList
instead ofSeries
and update related query functions.getProducerThroughputOverview
inhttp_handler.go
to useList
instead ofSeries
for data handling.latencyColumn
to useList
instead ofSeries
.service_name
andtopic
.PanelType
fromTable
toList
inBuildQRParamsWithCache
andbuildCompositeQuery
intranslator.go
for specific contexts.producer-throughput-overview-latency
inbuildCompositeQuery
.http_handler.go
to align with new data structure.This description was created by for bf74ac7. It will automatically update as commits are pushed.