-
Notifications
You must be signed in to change notification settings - Fork 369
Added support to send different headers, apiRoot and different query param values to candidate, primary and secondary servers #43
base: master
Are you sure you want to change the base?
Conversation
hi @puneetkhanduri these two tests failed on my local machine too. But I donot know why. The changes I have done should not effect those test cases. |
Ok figured out why the test cases were failing. While I was treating the request as HttpRequest the test case was sending string. Added a check to execute my code only when the request is an HttpRequest. Test cases are passing now. Willc ommit soon. |
… only when the request is an instance of httprequest
hi @puneetkhanduri I think now the coverage test is failing. Don't understand why from the logs though. |
Current coverage is 36.59%
@@ master #43 diff @@
==========================================
Files 29 29
Lines 789 850 +61
Methods 772 833 +61
Messages 0 0
Branches 17 17
==========================================
+ Hits 288 311 +23
- Misses 501 539 +38
Partials 0 0
|
@@ -3,15 +3,95 @@ package com.twitter.diffy.proxy | |||
import com.twitter.finagle.Service | |||
import com.twitter.util.{Future, Try} | |||
|
|||
class SequentialMulticastService[-A, +B]( | |||
services: Seq[Service[A, B]]) | |||
import org.jboss.netty.handler.codec.http.HttpRequest |
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.
This generic provides a common protocol-agnostic abstraction to multicast something to an arbitrary number of destinations. The concern of having target specific headers belongs to the HttpDifferenceProxy implementation and should live there.
Further, instead of creating 3 different versions of the request before sending any of them to the targets perhaps you could compose each target with a Filter that independently re-writes whatever requests it receives before forwarding it to the underlying target.
I will try to find some time and do the changes suggested |
@sukalpomitra This PR is still open. Did you figure out a different way to send headers? I am trying out diffy and I met with this requirement of sending headers. cc @puneetkhanduri |
@savakarv I moved on to other projects and did not have time to fix it. Why dont you fork mine and continue. |
sukalpo.mitra seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
You can send different HTTP Header Key value pairs to candidate, primary and secondary servers. Add
-candidateHeaders=Authorization:Basic OtxGHYUI, userRole:admin1 -primaryHeaders=Authorization:Basic NjhmskT, userRole:admin2 -secondaryHeaders=Authorization:Basic Tysfdg, userRole:admin3
to your command line argumentsYou can add apiRoots to the api calls to candidate, primary and secondary servers. Add
-candidateApiRoot="api/v2" -primaryApiRoot="api/v1" -secondaryApiroot="api/v1"
to your command line arguments. Usage:-curl 'localhost:8880/getAllMessages'
will send a requestcandidateServer:candidatePort/api/v2/getAllMessages
to candidate server while it will sendprimaryServer:primaryPort/api/v1/getAllMessages
to primary serverYou can now substitute different queryParam values before multicasting to the three different servers. Usage:-
curl 'localhost:8880/api/getUserMessages?message=userId'
. Create a file apiParams.xml with the following structure<apiParams> <userId> <candidate> 12 </candidate> <primary> 13 </primary> <secondary> 14 </secondary> </userId> </apiParams>
This will send a request
candidateServer:candidatePort/api/getUserMessages?message=12
to candidate server while sendingprimaryServer:primaryPort/api/getUserMessages?message=13
andsecondaryServer:secondaryPort/api/getUserMessages?message=14
request to primary and secondary servers respectively.