plusplusjiajia commented on code in PR #616:
URL: https://github.com/apache/iceberg-cpp/pull/616#discussion_r3253850443


##########
src/iceberg/catalog/rest/http_client.cc:
##########
@@ -68,27 +70,37 @@ namespace {
 /// \brief Default error type for unparseable REST responses.
 constexpr std::string_view kRestExceptionType = "RESTException";
 
-/// \brief Prepare headers for an HTTP request.
-Result<cpr::Header> BuildHeaders(
-    const std::unordered_map<std::string, std::string>& request_headers,
+/// \brief Merge default headers with per-request headers (per-request wins).
+std::unordered_map<std::string, std::string> MergeHeaders(
     const std::unordered_map<std::string, std::string>& default_headers,
-    auth::AuthSession& session) {
-  std::unordered_map<std::string, std::string> headers(default_headers);
+    const std::unordered_map<std::string, std::string>& request_headers) {
+  std::unordered_map<std::string, std::string> merged(default_headers);
   for (const auto& [key, val] : request_headers) {
-    headers.insert_or_assign(key, val);
+    merged.insert_or_assign(key, val);
   }
-  ICEBERG_RETURN_UNEXPECTED(session.Authenticate(headers));
-  return cpr::Header(headers.begin(), headers.end());
+  return merged;
+}
+
+cpr::Header ToCprHeader(const auth::HTTPRequest& request) {
+  return {request.headers.begin(), request.headers.end()};
 }
 
-/// \brief Converts a map of string key-value pairs to cpr::Parameters.
-cpr::Parameters GetParameters(
+/// \brief Append URL-encoded query parameters to a URL, sorted by key.
+Result<std::string> AppendQueryString(
+    const std::string& base_url,
     const std::unordered_map<std::string, std::string>& params) {
-  cpr::Parameters cpr_params;
-  for (const auto& [key, val] : params) {
-    cpr_params.Add({key, val});
+  if (params.empty()) return base_url;
+  std::map<std::string, std::string> sorted(params.begin(), params.end());
+  std::string url = base_url + "?";

Review Comment:
   @wgtmac Thanks! Added a doc comment: base_url must not already contain a 
query string (? or &).



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