Skip to content
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

return empty series if metrics exist in index and has no points #240

Merged
merged 10 commits into from
Jul 27, 2023

Conversation

lordvidex
Copy link
Contributor

@lordvidex lordvidex commented Jul 10, 2023

return empty series if metrics exist in index and has no points

Refs: #178

* fix: NaN is filled when index has no point

- it is always filled either other index result has points or not
@lordvidex lordvidex changed the title fix: NaN is filled when index result has no points feat: NaN is filled when index result has no points Jul 11, 2023
- step1: test is failing
- step2: tests are about 60% complete
- increased test coverage for `render` from 29.1% to 69.1%
msaf1980

This comment was marked as duplicate.

msaf1980

This comment was marked as duplicate.

// result when CHResponse.AppendOutEmptySeries is false
expected0 []client.Metric
// result when CHResponse.AppendOutEmptySeries is true
expected1 []client.Metric
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be better expectedWithoutEmpty and expectedWithEmpty instead of magic names expected0 / expected1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using names like this, but they sounded ambiguous too. In this case,
expected0 == expectedWithEmpty ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected0 => expectedWithoutEmpty ?

render/reply/formatter_test.go Outdated Show resolved Hide resolved
render/reply/protobuf.go Show resolved Hide resolved
render/data/ch_response.go Outdated Show resolved Hide resolved
render/reply/pickle.go Outdated Show resolved Hide resolved
@lordvidex lordvidex requested a review from msaf1980 July 14, 2023 20:03
@@ -90,23 +93,8 @@ func (cc *CHResponses) ToMultiFetchResponseV2() (*v2pb.MultiFetchResponse, error
func (c *CHResponse) ToMultiFetchResponseV3() (*v3pb.MultiFetchResponse, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToMultiFetchResponseV2 also need changes for Null Points like ToMultiFetchResponseV3

// result when CHResponse.AppendOutEmptySeries is true
expected1 []client.Metric
expectedWithoutEmpty []client.Metric
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be naming mistake and expectedWithoutEmpty <-> expectedWithtEmpty ?

// result when CHResponse.AppendOutEmptySeries is true
expected1 []client.Metric
expectedWithoutEmpty []client.Metric

@msaf1980 msaf1980 changed the title feat: NaN is filled when index result has no points return empty series if metrics exist in index and has no points Jul 27, 2023
@msaf1980 msaf1980 merged commit dd45bed into go-graphite:master Jul 27, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants