You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "singhpk234 (via GitHub)" <gi...@apache.org> on 2025/06/04 02:42:21 UTC
[PR] REST: Add UA [iceberg]
singhpk234 opened a new pull request, #13234:
URL: https://github.com/apache/iceberg/pull/13234
### About the change
This change attempts to add UA so that clients can set this at their end and it can supplement the server for audit and debugging purpose, we do have `X-Client-Version` which helps solve most of the cases as this helps server understand that what version of the client the server is interacted, but in a world where we want to see the logs at server end and identify what happened to a client will be very hard.
Putting this change out to see if folks are interested in this !
### Testing
TODO
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] REST: Add property for configuring user agent in http client [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #13234:
URL: https://github.com/apache/iceberg/pull/13234#discussion_r2129822572
##########
core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java:
##########
@@ -461,7 +466,8 @@ private static String addRequestTestCaseAndGetPath(HttpMethod method, Item body,
.withMethod(method.name().toUpperCase(Locale.ROOT))
.withHeader("Authorization", "Bearer " + BEARER_AUTH_TOKEN)
.withHeader(HTTPClient.CLIENT_VERSION_HEADER, icebergBuildFullVersion)
- .withHeader(HTTPClient.CLIENT_GIT_COMMIT_SHORT_HEADER, icebergBuildGitCommitShort);
+ .withHeader(HTTPClient.CLIENT_GIT_COMMIT_SHORT_HEADER, icebergBuildGitCommitShort)
+ .withHeader(USER_AGENT, TEST_USER_AGENT);
Review Comment:
precisely, request level override always wins over the client level override.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] REST: Add property for configuring user agent in REST http client [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #13234:
URL: https://github.com/apache/iceberg/pull/13234#discussion_r2129802487
##########
core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java:
##########
@@ -461,7 +466,8 @@ private static String addRequestTestCaseAndGetPath(HttpMethod method, Item body,
.withMethod(method.name().toUpperCase(Locale.ROOT))
.withHeader("Authorization", "Bearer " + BEARER_AUTH_TOKEN)
.withHeader(HTTPClient.CLIENT_VERSION_HEADER, icebergBuildFullVersion)
- .withHeader(HTTPClient.CLIENT_GIT_COMMIT_SHORT_HEADER, icebergBuildGitCommitShort);
+ .withHeader(HTTPClient.CLIENT_GIT_COMMIT_SHORT_HEADER, icebergBuildGitCommitShort)
+ .withHeader(USER_AGENT, TEST_USER_AGENT);
Review Comment:
Just for my understanding, if I have a HTTP client configured to "my-user-agent" and then I later set it to "some-new-agent" on the request level, it will override it at the request level right? Just wanted to double check the order of precedence (not that I expect it to be used in practice that way)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] REST: Add property for configuring user agent in http client [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #13234:
URL: https://github.com/apache/iceberg/pull/13234#discussion_r2129822572
##########
core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java:
##########
@@ -461,7 +466,8 @@ private static String addRequestTestCaseAndGetPath(HttpMethod method, Item body,
.withMethod(method.name().toUpperCase(Locale.ROOT))
.withHeader("Authorization", "Bearer " + BEARER_AUTH_TOKEN)
.withHeader(HTTPClient.CLIENT_VERSION_HEADER, icebergBuildFullVersion)
- .withHeader(HTTPClient.CLIENT_GIT_COMMIT_SHORT_HEADER, icebergBuildGitCommitShort);
+ .withHeader(HTTPClient.CLIENT_GIT_COMMIT_SHORT_HEADER, icebergBuildGitCommitShort)
+ .withHeader(USER_AGENT, TEST_USER_AGENT);
Review Comment:
precisely, request level override always wins the client level override.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] REST: Add property for configuring user agent in http client [iceberg]
Posted by "ebyhr (via GitHub)" <gi...@apache.org>.
ebyhr commented on code in PR #13234:
URL: https://github.com/apache/iceberg/pull/13234#discussion_r2131256596
##########
core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java:
##########
@@ -71,6 +71,8 @@ public class TestHTTPClient {
private static final int PORT = 1080;
private static final String BEARER_AUTH_TOKEN = "auth_token";
+ private static final String USER_AGENT = "User-Agent";
Review Comment:
nit: `USER_AGENT` constant exists in `HttpHeaders` class.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] REST: Add property for configuring user agent in http client [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #13234:
URL: https://github.com/apache/iceberg/pull/13234#discussion_r2145887045
##########
core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java:
##########
@@ -71,6 +71,8 @@ public class TestHTTPClient {
private static final int PORT = 1080;
private static final String BEARER_AUTH_TOKEN = "auth_token";
+ private static final String USER_AGENT = "User-Agent";
Review Comment:
btw, I didn't find the constant in the class `org.apache.iceberg.rest.HttpHeaders`. Are you referring to another class with the same name?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] REST: Add property for configuring user agent in http client [iceberg]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #13234:
URL: https://github.com/apache/iceberg/pull/13234#discussion_r2145862007
##########
core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java:
##########
@@ -71,6 +71,8 @@ public class TestHTTPClient {
private static final int PORT = 1080;
private static final String BEARER_AUTH_TOKEN = "auth_token";
+ private static final String USER_AGENT = "User-Agent";
Review Comment:
I was thinking about adding this constant to class `HTTPClient`, but if it's only used by test, we might not do that.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] REST: Add property for configuring user agent in http client [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar merged PR #13234:
URL: https://github.com/apache/iceberg/pull/13234
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org