HttpSink Plugin Functionality for #874. #3036
Conversation
Signed-off-by: mallikagogoi7 <mallikagogoi7@gmail.com>
mallikagogoi7
requested review from
chenqi0805,
engechas,
graytaylor0,
dinujoh,
kkondaka,
cmanning09,
asifsmohammed,
dlvenable and
oeyh
as code owners
| @@ -240,4 +248,12 @@ public String getDlqFile() { | |||
| public PluginModel getDlq() { | |||
| return dlq; | |||
| } | |||
|
|
|||
| public boolean isValidAWSUrl() { | |||
There was a problem hiding this comment.
What is the point of this code? I also don't see it in use. Why does AWS have a special case here?
There was a problem hiding this comment.
Thanks for the review @dlvenable , this AWS url validation check is done before attaching the AWS Sigv4.
| } | ||
|
|
||
| if(Objects.nonNull(httpSinkConfiguration.getCustomHeaderOptions())) | ||
| addSageMakerHeaders(classicRequestBuilder,httpSinkConfiguration.getCustomHeaderOptions()); |
There was a problem hiding this comment.
Why does setting "custom_headers" create Sage Maker options?
Custom header should be a Map<String, List<String>> which allows users to define any headers they want.
There was a problem hiding this comment.
Thanks for the review @dlvenable , changed the custom header to Map<String, List
| " http_method: \"POST\"\n" + | ||
| " auth_type: \"http_basic\"\n" + | ||
| private static final String SINK_YAML = | ||
| " url: \"https://httpbin.org/post\"\n" + |
There was a problem hiding this comment.
Please test with a localhost address instead of this webpage.
There was a problem hiding this comment.
Thanks for the review @dlvenable , changed the url to localhost:8080.
| final String proxyUrlString = httpSinkConfiguration.getProxy(); | ||
| final ClassicRequestBuilder classicRequestBuilder = buildRequestByHTTPMethodType(httpMethod).setUri(httpSinkConfiguration.getUrl()); | ||
|
|
||
| if(httpSinkConfiguration.isAwsSigv4() && httpSinkConfiguration.isValidAWSUrl()){ |
There was a problem hiding this comment.
Let's split all of this AWS code into its own class. It is too tightly coupling the code.
There was a problem hiding this comment.
Thanks for the review @dlvenable , the AWS code is moved to a new class.
Signed-off-by: mallikagogoi7 <mallikagogoi7@gmail.com>
Signed-off-by: mallikagogoi7 <mallikagogoi7@gmail.com>
Signed-off-by: mallikagogoi7 <mallikagogoi7@gmail.com>
| if (ThresholdValidator.checkThresholdExceed(currentBuffer, maxEvents, maxBytes, maxCollectionDuration)) { | ||
| final HttpEndPointResponse failedHttpEndPointResponses = pushToEndPoint(getCurrentBufferData(currentBuffer)); | ||
| if (failedHttpEndPointResponses != null) { | ||
| logFailedData(failedHttpEndPointResponses, getCurrentBufferData(currentBuffer)); |
There was a problem hiding this comment.
It should be releaseEventHandle(false) if it fails to push to Http and no DLQ is configured OR DLQ configured but failed to write to DLQ
Description
HttpSink Plugin Functionality for #874.
Issues Resolved
#874
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.