adutra commented on code in PR #12595:
URL: https://github.com/apache/iceberg/pull/12595#discussion_r2024339056
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -332,6 +343,46 @@ public void testInitializeWithBadArguments() throws
IOException {
restCat.close();
}
+ @Test
+ public void testDefaultHeadersPropagated() throws IOException {
Review Comment:
If we want to go one step further, we could test that the config headers
arrive at the servlet and contain the expected values:
https://github.com/adutra/iceberg/commit/7dc230afaef28a39b647d6e678405dd827a03017
Again feel free to cherry-pick if you agree :-)
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -332,6 +343,46 @@ public void testInitializeWithBadArguments() throws
IOException {
restCat.close();
}
+ @Test
+ public void testDefaultHeadersPropagated() throws IOException {
Review Comment:
It's not clear to me what this test is trying to verify.
First off, I think it's important to use _the default constructor_ of
`RESTCatalog`, since that's the constructor we want to exercise. The other
constructors give users the power to do anything they want with the
`HTTPClient`, and because of that, I think they are out of scope here.
Next, the goal would be to assert that the `HTTPClient` created by the
default `RESTCatalog` constructor will propagate the config headers.
I think the below test is therefore enough:
```java
@Test
public void testDefaultHeadersPropagated() {
RESTCatalog catalog = new RESTCatalog();
Map<String, String> properties =
Map.of(
CatalogProperties.URI,
httpServer.getURI().toString(),
OAuth2Properties.CREDENTIAL,
"catalog:secret",
"header.X-Iceberg-Access-Delegation",
"vended-credentials");
catalog.initialize("test", properties);
assertThat(catalog)
.extracting("sessionCatalog.client.baseHeaders")
.asInstanceOf(map(String.class, String.class))
.containsEntry("X-Iceberg-Access-Delegation", "vended-credentials");
}
```
Feel free to cherry-pick this commit if that makes sense:
https://github.com/adutra/iceberg/commit/0c657f22bdb21417141e892643d6f7eb5a931484
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
