-
Notifications
You must be signed in to change notification settings - Fork 18
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
PoC using redirect #16
PoC using redirect #16
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16 +/- ##
==========================================
+ Coverage 19.40% 20.31% +0.90%
==========================================
Files 9 9
Lines 438 507 +69
==========================================
+ Hits 85 103 +18
- Misses 341 391 +50
- Partials 12 13 +1
Continue to review full report at Codecov.
|
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.
LGTM, but the results will need to be handled correctly later
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 overall, I just don't like listing endpoints in each service, probably we could use prefixes, like
/api/v1/aggregator/*
-> redirect to aggregator service
/api/v1/content/*
-> redirect to content service
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.
LGTM, I don't see any significant issue 👍
server/server.go
Outdated
} | ||
|
||
// genericHandle | ||
func (server HTTPServer) genericContentServiceRedirect(writer http.ResponseWriter, request *http.Request) { |
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.
btw, genericAggregatorRedirect, and genericContentServiceRedirect are almost the same function, so maybe it could be something like
func (server HTTPServer) redirectTo(baseURL string) func(http.ResponseWriter, *http.Request)
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.
Done
the whole redirect things are basically throwaway code anyway |
…ng function to cache instead redirect
I added a new function able to perform the request to the aggregator/content-service and then response back to the client |
Description
Just a PoC for the smart-proxy service using HTTP redirects.
This solution doesn't cache anything, so the final smart-proxy will look very different from this implementation.
Fixes #15
Type of change
Testing steps
Regular CI, no new tests at the moment