Skip to content

REST: Add property for configuring user agent in http client #13234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amogh-jahagirdar merged 2 commits into from
Jun 13, 2025
Merged

REST: Add property for configuring user agent in http client #13234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a17de34
Add UA
sfc-gh-prsingh Jun 4, 2025
0ba6c09
Add tests
sfc-gh-prsingh Jun 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/src/main/java/org/apache/iceberg/rest/HTTPClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public class HTTPClient extends BaseHTTPClient {
static final String REST_PROXY_PORT = "rest.client.proxy.port";
static final String REST_PROXY_USERNAME = "rest.client.proxy.username";
static final String REST_PROXY_PASSWORD = "rest.client.proxy.password";
static final String REST_USER_AGENT = "rest.client.user-agent";

@VisibleForTesting
static final String REST_CONNECTION_TIMEOUT_MS = "rest.client.connection-timeout-ms";
Expand Down Expand Up @@ -116,6 +117,11 @@ private HTTPClient(
int maxRetries = PropertyUtil.propertyAsInt(properties, REST_MAX_RETRIES, 5);
clientBuilder.setRetryStrategy(new ExponentialHttpRequestRetryStrategy(maxRetries));

String userAgent = PropertyUtil.propertyAsString(properties, REST_USER_AGENT, null);
if (userAgent != null) {
clientBuilder.setUserAgent(userAgent);
}

if (proxy != null) {
if (proxyCredsProvider != null) {
clientBuilder.setDefaultCredentialsProvider(proxyCredsProvider);
Expand Down
10 changes: 8 additions & 2 deletions core/src/test/java/org/apache/iceberg/rest/TestHTTPClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Copy link
Member

@ebyhr ebyhr Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: USER_AGENT constant exists in HttpHeaders class.

Copy link
Contributor

@flyrain flyrain Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about adding this constant to class HTTPClient, but if it's only used by test, we might not do that.

Copy link
Contributor

@flyrain flyrain Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

private static final String TEST_USER_AGENT = "Test-User-Agent";
private static final String URI = String.format("http://127.0.0.1:%d", PORT);
private static final ObjectMapper MAPPER = RESTObjectMapper.mapper();

Expand All @@ -83,7 +85,10 @@ public class TestHTTPClient {
public static void beforeClass() {
mockServer = startClientAndServer(PORT);
restClient =
HTTPClient.builder(ImmutableMap.of()).uri(URI).withAuthSession(AuthSession.EMPTY).build();
HTTPClient.builder(ImmutableMap.of(HTTPClient.REST_USER_AGENT, TEST_USER_AGENT))
.uri(URI)
.withAuthSession(AuthSession.EMPTY)
.build();
icebergBuildGitCommitShort = IcebergBuild.gitCommitShortId();
icebergBuildFullVersion = IcebergBuild.fullVersion();
}
Expand Down Expand Up @@ -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);
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

@singhpk234 singhpk234 Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

precisely, request level override always wins over the client level override.


if (method.usesRequestBody()) {
mockRequest = mockRequest.withBody(asJson);
Expand Down