smaheshwar-pltr commented on code in PR #1090:
URL: https://github.com/apache/iceberg-cpp/pull/1090#discussion_r4474764001


##########
src/iceberg/catalog/rest/http_client.cc:
##########
@@ -34,19 +37,25 @@ namespace iceberg::rest {
 
 class HttpResponse::Impl {
  public:
-  explicit Impl(cpr::Response&& response) : response_(std::move(response)) {}
+  explicit Impl(cpr::Response response)
+      : status_code_(static_cast<int32_t>(response.status_code)),
+        body_(std::move(response.text)),
+        headers_(response.header.begin(), response.header.end()) {}
   ~Impl() = default;
 
-  int32_t status_code() const { return 
static_cast<int32_t>(response_.status_code); }
+  static Result<HttpResponse> Make(cpr::Response response,
+                                   const ErrorHandler& error_handler);
 
-  std::string body() const { return response_.text; }
+  int32_t status_code() const { return status_code_; }
 
-  std::unordered_map<std::string, std::string> headers() const {
-    return {response_.header.begin(), response_.header.end()};
-  }
+  std::string body() const { return body_; }
+
+  std::unordered_map<std::string, std::string> headers() const { return 
headers_; }
 
  private:
-  cpr::Response response_;
+  int32_t status_code_;
+  std::string body_;
+  std::unordered_map<std::string, std::string> headers_;

Review Comment:
   The changes in this class are due to this. The cpr::Response maintains a 
curl handle so destruction affects the pool. I'm therefore instead storing 
what's needed explicitly. See below / PR description for more details



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to