Skip to content

Conversation

@dramaticlly
Copy link
Contributor

Attempt to revive #12194 to expose the response header on GET requests in RESTClient

@nastra @danielcweeks can you help take a look?

Co-authored-by: Gabor Kaszab <[email protected]>
Signed-off-by: Hongyue Zhang <[email protected]>
Comment on lines +298 to +305
if (responseHeaders != null) {
responseHeaders.accept(ImmutableMap.of("X-Request-Id", String.valueOf(System.nanoTime())));

// Set route specific response headers
switch (route) {
case CONFIG:
responseHeaders.accept(ImmutableMap.of("X-Iceberg-Version", IcebergBuild.version()));
}
Copy link
Contributor Author

@dramaticlly dramaticlly Jul 31, 2025

Choose a reason for hiding this comment

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

I realize this is public class in test package and shared in many tests, want to leverage this to ensure we cover the basic test of response header. Please let me know if there's better place to set response headers before features like idempotent etag is implemented.

@gaborkaszab
Copy link
Collaborator

gaborkaszab commented Jul 31, 2025

Hey @dramaticlly ,
This PR seems almost identical to #12194 I had except this adds X-Iceberg-Version and X-Iceberg-Version response headers. I still have the ETag and freshness aware loading support on my plate, so I'd prefer to re-open the original PR for the response header support and then I'll resume my work on the rest of the functionality.
Anyway, due to timezones I didn't have much chance to respond to your question on the other PR.

@dramaticlly
Copy link
Contributor Author

Hey @dramaticlly ,

This PR seems almost identical to #12194 I had except this adds X-Iceberg-Version and X-Iceberg-Version response headers. I still have the ETag and freshness aware loading support on my plate, so I'd prefer to re-open the original PR for the response header support and then I'll resume my work on the rest of the functionality.

Anyway, due to timezones I didn't have much chance to respond to your question on the other PR.

Sure @gaborkaszab, I added you as coauthor in the commit as most of the code is in attempt to revive the work. I think it would be the best if you want to continue the original PR. I can close this one

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.

2 participants