aws/aws-sdk-cpp

S3 client ignores http configuration

kiwaa opened this issue · 5 comments

kiwaa commented

Describe the bug

S3Client::operator= was introduced in the commit which does not properly reinitialise base class. m_httpclient uses default timeouts in this case

Expected Behavior

Configuration (and timeouts) are respected. Code below outputs 30000

Current Behavior

Timeouts are not respected as inner m_httpclient is initialised with default configuration. Code below prints ~150

Reproduction Steps

Aws::SDKOptions options;
Aws::InitAPI(options);
{
    Aws::Client::ClientConfiguration clientConfig;
    clientConfig.connectTimeoutMs = 30'000;
    clientConfig.httpRequestTimeoutMs = 30'000;
    clientConfig.requestTimeoutMs = 30'000;
    clientConfig.proxyHost = "localhost";
    clientConfig.proxyPort = 1234;

    // ok
    // Aws::S3::S3Client client = Aws::S3::S3Client(clientConfig);

    // not ok since 1.11.212
    Aws::S3::S3Client client;
    client = Aws::S3::S3Client(clientConfig);

    Aws::S3::Model::GetObjectRequest request;
    request.SetBucket("test_bucket");
    request.SetKey("test_key");

    auto start = std::chrono::system_clock::now();
    auto outcome = client.GetObject(request);
    auto end = std::chrono::system_clock::now();

    auto now_ms = std::chrono::duration_cast<std::chrono::milliseconds>(end - start);

    std::cout << now_ms.count() << std::endl;
}
Aws::ShutdownAPI(options);

Possible Solution

No response

Additional Information/Context

No response

AWS CPP SDK version used

1.11.212

Compiler and Version used

Visual Studio

Operating System and version

Windows 11

DmitriyMusatkin commented

should have been fixed with 7e7ed06, that got released in 1.11.233.
Can you try the latest version?

kiwaa commented

sorry, I should have been more specific. It was tested on 1.11.331 and I just tried on the latest master - still doesn't work.
From the commit that you provided, I don't see changes around m_httpclient (in AWSClient or S3Client).

S3 client was built and tested with curl, but from the code I don't think it matters

jmklix commented

This should be fixed with this PR: #2977
Please update to the latest version of this sdk and let us know if you run into any problems

kiwaa commented

Confirmed, it is fixed now. thanks!

github-actions commented

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.