Skip to content

Conversation

@cleaton
Copy link
Contributor

@cleaton cleaton commented May 11, 2025

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

@COM8
Copy link
Member

COM8 commented May 13, 2025

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

@COM8
Copy link
Member

COM8 commented May 13, 2025

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

@COM8
Copy link
Member

COM8 commented Jun 14, 2025

@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?

@cleaton
Copy link
Contributor Author

cleaton commented Jun 14, 2025

@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
Copy link
Contributor Author

cleaton commented Jun 15, 2025

I've updated PR and added documentation: libcpr/docs#57

@COM8
Copy link
Member

COM8 commented Jun 22, 2025

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

@COM8
Copy link
Member

COM8 commented Jun 22, 2025

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
Copy link
Member

COM8 commented Jul 11, 2025

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

@COM8 COM8 merged commit e10e86f into libcpr:master Jul 11, 2025
67 of 69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants