-
Notifications
You must be signed in to change notification settings - Fork 123
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
Make empty results ok #453
Conversation
The DeepSource failing appears to be from unrelated code. |
@jdblack : could you also please apply go fmt ? linter also complains - https://github.com/go-graphite/go-carbon/runs/5535152765?check_suite_focus=true |
@jdblack : OT : and I'm joining to @bom-d-van question too - are you really running Graphite cluster with 3B metrics, as you mentioned in #424 (comment) ??? |
Thanks for the PR review! I fixed the conditional as asked. I ran "gofmt -w -s" on the golang files that I touched, and none seemed modified. A "gofmt -w -s ." showed changes to 4 unrelated files, mostly related to the trie code. Would you like me to gofmt those? Yes, in our biggest environment I do mean to answer @bom-d-van, but time got away from me on Friday. I promise I'll put something together early this week, but here's a pic =) https://imagebin.ca/v/6aCg0IgCamRF . |
@jdblack : now everything looks good, thanks! |
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, lgtm as well. the deepsource complain isn't introduced in this pr. so I guess we can skip it. (will open a ticket to track it, seems a reasonable optimisation suggestion). thanks for the new feature.
Yes, in our biggest environment I do mean to answer @bom-d-van, but time got away from me on Friday. I promise I'll put something together early this week, but here's a pic =) https://imagebin.ca/v/6aCg0IgCamRF .
wow, 3 billions... that's nuts. 👍
Carbonserver currently returns 400 if the result set is empty. Graphite-web handles this result poorly, resulting in graphite-web returning 500 series errors to grafana. This PR provides an option in go-carbon which configures gocarbon to return a 200 result if the result set is empty.