Fix curl callback data latency for clients with response stream #3245
Conversation
sbera87
changed the title
| @@ -203,6 +203,11 @@ namespace Aws | |||
|
|
|||
| virtual Aws::Client::CompressionAlgorithm | |||
| GetSelectedCompressionAlgorithm(Aws::Client::RequestCompressionConfig) const { return Aws::Client::CompressionAlgorithm::NONE; } | |||
| /* | |||
There was a problem hiding this comment.
please move it up right after IsEventStreamRequest
please also use a consistent docstring style of this file
/**
* Defaults to false, if this is set to true in derived class, the operation using this request will return an event stream response.
*/
inline virtual bool HasEventStreamResponse() const { return false; }
| @@ -289,7 +289,7 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, | |||
| false/*retryable*/)); | |||
|
|
|||
| }; | |||
| httpRequest->SetEventStreamRequest(request.IsEventStreamRequest()); | |||
| httpRequest->SetEventStreamRequest(request.IsEventStreamRequest() || request.HasEventStreamResponse()); | |||
There was a problem hiding this comment.
Please don't overload IsEventStreamRequest boolean in the HttpRequest object, it may create unwanted side-effects of how the request is sent.
Instead, create a new boolean flag at set it accordingly.
|
|
||
| void WriteDataToStream(Aws::Kinesis::KinesisClient &kinesis_client, const std::string &streamName, const std::string &data, const std::string &partitionKey) | ||
| { | ||
| std::cout<<"WriteDataToStream called"<<std::endl; |
There was a problem hiding this comment.
please remove debug traces, our tests typically do not trace to stdout on success.
you can use SCOPED_TRACE() macro of gtest to add additional context to the test that will be dumped to stdout on failure.
| if (putRecordOutcome.IsSuccess()) { | ||
| std::cout << "Successfully put record into stream: " << data << std::endl; | ||
| } else { | ||
| std::cout << "Error putting record into stream: " |
There was a problem hiding this comment.
shouldn't we fail the test in this case and don't proceed further?
| @@ -13,7 +13,7 @@ int main(int argc, char** argv) | |||
| { | |||
| Aws::Testing::SetDefaultSigPipeHandler(); | |||
| Aws::SDKOptions options; | |||
| options.loggingOptions.logLevel = Aws::Utils::Logging::LogLevel::Trace; | |||
| options.loggingOptions.logLevel = Aws::Utils::Logging::LogLevel::Debug; | |||
There was a problem hiding this comment.
I don't think this is a part of this change.
| @@ -291,6 +291,10 @@ protected SdkFileEntry generateModelHeaderFile(ServiceModel serviceModel, Map.En | |||
| if (op.getRequest() != null && op.getRequest().getShape().getName() == shape.getName()) { | |||
| context.put("operation", op); | |||
| context.put("operationName", key); | |||
| if((op.getResult() != null) && op.getResult().getShape().hasEventStreamMembers()) | |||
There was a problem hiding this comment.
java does not use K&R C curly braces format.
Issue #, if available:
Description of changes:
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.