Skip to content

frontend: [Refactor] Isolated middlewares from frontend to allow usage from external projects. #1437

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 31, 2019
Merged

frontend: [Refactor] Isolated middlewares from frontend to allow usage from external projects. #1437

merged 5 commits into from
Jul 31, 2019

Conversation

tomwilkie
Copy link
Contributor

(Resurrecting #1332)

@tomwilkie tomwilkie force-pushed the middlewares branch 7 times, most recently from 95a9581 to a0ce758 Compare June 2, 2019 19:32
@tomwilkie
Copy link
Contributor Author

The three bits of feedback on the original PR were:

  • Give reasons for the overrides in dep: We don't use dep anymore, although I've still updated client_golang to the latest release which pulled in some extra stuff.
  • We haven't traditionally checked in the .pb.go files: We do now.
  • Could you split the commit into one which purely updates vendor: Done, and split out the change which moved the files to make any edits more obvious.

@tomwilkie
Copy link
Contributor Author

@gouthamve @bwplotka mind taking a look?

@tomwilkie tomwilkie requested a review from gouthamve July 15, 2019 16:15
gouthamve
gouthamve approved these changes Jul 17, 2019
View reviewed changes
Copy link
Contributor

@gouthamve gouthamve left a comment

Choose a reason for hiding this comment

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

LGTM

tomwilkie and others added 5 commits July 31, 2019 11:21
@tomwilkie
Update prometheus/client_golang.
0f9c172 
Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie
Move frontend middlewares into new package.
6416c87 
Signed-off-by: Tom Wilkie <[email protected]>
@bwplotka @tomwilkie
frontend: (Refactor) Isolated middlewares from frontend to allow usag…
8d29802 
…e from external projects.

No logic should be changed, just refactor that allows middlewares reuse.

Changes:
* frontend.proto only specifies gRPC service
* queryrange.proto has rest (query API for range) and is in queryrange package.
* Removed util.Logger from queryrange
* Removed usages of overrides, moved to Limits interface in queryrange.
* Some namings.
* Removed log messages there were duplicated with return error (bad error handling).
* Added TODOs
* ResultCache Middleware has now config independent constructor.

TODO in followng PRs: Retry middleware & other TODOs

Signed-off-by: Bartek Plotka <[email protected]>
Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie
More refactoring, including:
e6d6301 
- Get it all to build, test and lint
- Make some methods package private, add comments to others.
- Update the Prometheus metrics autoscalling code to embed the interface it is implementing, and remove the "not implemented" methods.  This technique is much less brittle.
- Separate out the retry logic into middleware.
- Revert some renames.
- Don't deeply nest packages.
- Fix imports from move.
- Tidy up retry middleward and tests.

Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie
Fixes after rebase, and review comments.
037f4c9 
Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie tomwilkie merged commit 900ca62 into master Jul 31, 2019
@tomwilkie tomwilkie deleted the middlewares branch July 31, 2019 10:53
@gouthamve gouthamve mentioned this pull request Apr 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gouthamve gouthamve gouthamve approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tomwilkie @gouthamve @bwplotka