-
Notifications
You must be signed in to change notification settings - Fork 4
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
[feature] Distributed query infrastructure (global-query server) #109
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.
Obviously no guarantees to safety of the review (my head's spinning after going through this behemoth of a PR 💫 ).
Change requested because of the raw query issue (not sure if others are affected, too). I'll try to help investigate as much as I can.
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.
Should this maybe go someplace else (e.g. addon
or examples
or similar)?
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.
I like to keep it next to the binary, in case someone wants to run the tool directly inside the directory with go run main.go --config ....
.
But no strong opinion here.
cmd/global-query/local-config.yaml
Outdated
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.
Should this maybe go someplace else (e.g. addon
or examples
or similar)?
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.
We could. Then we should supply them for every binary though.
Thanks for the initial review and the suggestions. I'll also prioritize looking at the raw queries. Unfortunately never ran them during testing 🙈. |
82c7a65
to
e4f95f4
Compare
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.
@fako1024 : I've violently broken the build by rebasing on develop. Now it's complaining about changes that come from slimcap
. Could you help me with this? I don't want to fix something that ain't broken.
pkg/api/client/client.go
Outdated
// DO NOT uncomment before clarifying behavior with httpc library | ||
// | ||
// retry any request that isn't 2xx | ||
// req = req.RetryBackOffErrFn(func(resp *http.Response, _ error) bool { |
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.
No problem. I really appreciate the conciseness of the library. At least 10x better than all the boilerplate repetition to make http requests.
Your suggestion with the interval generators sounds really good and simple to use. Then you don't have to modify the RetryBackOff
function signature and just provide some syntactic sugar on top. Nice!
Sure thing, I got your back! I've reverted |
Gets rid of the single invocation use for global-query and now requires the tool to run in server mode. Various other fixes and adjustments and preparation for a single api folder that stores all API clients and servers for the involved tools
Also adds a basic scaffold for setting tracing information (tbc).
… clients Resides somewhere in the RetryBackoff function of the httpc library.
@fako1024 : given that the review feedback has been incorporated and the major blocker (time queries not working) has been resolved, may I have your blessings? The ideas are storming into my head and I'd like to tick them off in more digestable chunks and in separate issues. The API restructure of goProbe, for example, is next. Toodles and hi to the old man. |
Sure thing, let's go ahead. Thanks for following up on the feedback. Side note: I've recently released |
Many thanks. Will do another bump in go.mod of httpc when reworking the API and clients. |
Time to let the cat out of the bag. This is the first working version with
It also moves the entire goProbe API into
legacyapi
, not serving it anymore. This is preparation for #58, which should then be a more concise PR than this beast.Closes #43