cpr icon indicating copy to clipboard operation
cpr copied to clipboard

Add connection pool option (V3)

Open cleaton opened this issue 6 months ago • 5 comments

Add connection pool option (see #488 & #330)

cleaton avatar May 11 '25 12:05 cleaton

@cleaton thanks for contributing! I will try to review it over the weekend.

COM8 avatar May 13 '25 15:05 COM8

Please also add a MR here for documentation: https://github.com/libcpr/docs

COM8 avatar May 13 '25 15:05 COM8

@cleaton thanks again for contributing this (again). Is there a specific reason you are not following up with it so we can go ahead and merge it?

COM8 avatar Jun 14 '25 13:06 COM8

@cleaton thanks again for contributing this (again). Is there a specific reason you are not following up with it so we can go ahead and merge it?

Thanks for the feedback, Sorry for the delay, I’ve been off on vacation. I hope to have some time to check this weekend or next. 👍

cleaton avatar Jun 14 '25 16:06 cleaton

I've updated PR and added documentation: https://github.com/libcpr/docs/pull/57

cleaton avatar Jun 15 '25 12:06 cleaton

Perfect, thanks! Now I'm on vacation. I will try to review it next week.

COM8 avatar Jun 22 '25 12:06 COM8

cppcheck is failing and it looks like there is a race condition with one test on windows?

  [ RUN      ] MultipleGetTests.PoolAsyncGetMultipleTest
  D:\a\cpr\cpr\test\connection_pool_tests.cpp(55): error: Expected equality of these values:
    std::string{"Hello world!"}
      Which is: "Hello world!"
    response.text
      Which is: ""
  
  D:\a\cpr\cpr\test\connection_pool_tests.cpp(57): error: Expected equality of these values:
    std::string{"text/html"}
      Which is: "text/html"
    response.header["content-type"]
      Which is: ""
  
  D:\a\cpr\cpr\test\connection_pool_tests.cpp(58): error: Expected equality of these values:
    200
    response.status_code
      Which is: 0
  
  D:\a\cpr\cpr\test\connection_pool_tests.cpp(60): error: Expected equality of these values:
    server->GetConnectionCount()
      Which is: 99
    NUM_REQUESTS
      Which is: 100
  
  [  FAILED  ] MultipleGetTests.PoolAsyncGetMultipleTest (2553 ms)
  [----------] 2 tests from MultipleGetTests (2596 ms total)

COM8 avatar Jun 22 '25 12:06 COM8

@cleaton thanks for taking care of this. Now we can merge it.

COM8 avatar Jul 11 '25 08:07 COM8