andrew4699 commented on code in PR #590:
URL: https://github.com/apache/polaris/pull/590#discussion_r1909321366


##########
integration-tests/src/main/java/org/apache/polaris/service/it/env/PolarisApiClient.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.it.env;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.PropertyNamingStrategies;
+import jakarta.ws.rs.client.Client;
+import jakarta.ws.rs.client.ClientBuilder;
+import jakarta.ws.rs.ext.ContextResolver;
+import java.io.Serializable;
+import java.net.URI;
+import org.apache.iceberg.rest.RESTSerializers;
+import org.apache.polaris.core.admin.model.PrincipalWithCredentials;
+
+public final class PolarisApiClient implements Serializable, AutoCloseable {

Review Comment:
   I think this and the TestEnvironment.id are where we have to think 
explicitly about what remote tests should look like. Live deployments are not 
quite the same as Polaris OSS for multiple reasons (forks, networking, 
security, DNS, base URL differences, etc). Still, Polaris contains a lot of 
tests and a subset of those tests are useful for validating a live Polaris 
deployment, hence the desire to re-use the same code.
   
   To re-use the tests there has to be a large degree of customizability. 
Providing a custom HTTP client gives an insane amount of customizability as you 
can intercept all requests/responses. If your company has an egress proxy, you 
can rewrite the URL to go through the proxy. You can wrap the request body too 
if your proxy requires that. You can use your own DNS resolution service. You 
can rewrite the base URL if your Polaris is hosted on a shared server. You can 
make simultaneous calls to an external service to authenticate and/or report 
metrics. This is effectively the route the Iceberg REST Catalog test suite has 
taken. You extend ViewTests to inherit the logic of 50+ test cases but you 
specify your own HTTP client.
   
   Technically, if the HTTP client can be specified, we don't even need the 
test ID since the HTTP client can just intercept everything and append an ID.



##########
integration-tests/src/main/java/org/apache/polaris/service/it/env/PolarisApiClient.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.service.it.env;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.PropertyNamingStrategies;
+import jakarta.ws.rs.client.Client;
+import jakarta.ws.rs.client.ClientBuilder;
+import jakarta.ws.rs.ext.ContextResolver;
+import java.io.Serializable;
+import java.net.URI;
+import org.apache.iceberg.rest.RESTSerializers;
+import org.apache.polaris.core.admin.model.PrincipalWithCredentials;
+
+public final class PolarisApiClient implements Serializable, AutoCloseable {

Review Comment:
   I think this and the `TestEnvironment.uniqueId` are where we have to think 
explicitly about what remote tests should look like. Live deployments are not 
quite the same as Polaris OSS for multiple reasons (forks, networking, 
security, DNS, base URL differences, etc). Still, Polaris contains a lot of 
tests and a subset of those tests are useful for validating a live Polaris 
deployment, hence the desire to re-use the same code.
   
   To re-use the tests there has to be a large degree of customizability. 
Providing a custom HTTP client gives an insane amount of customizability as you 
can intercept all requests/responses. If your company has an egress proxy, you 
can rewrite the URL to go through the proxy. You can wrap the request body too 
if your proxy requires that. You can use your own DNS resolution service. You 
can rewrite the base URL if your Polaris is hosted on a shared server. You can 
make simultaneous calls to an external service to authenticate and/or report 
metrics. This is effectively the route the Iceberg REST Catalog test suite has 
taken. You extend ViewTests to inherit the logic of 50+ test cases but you 
specify your own HTTP client.
   
   Technically, if the HTTP client can be specified, we don't even need the 
test ID since the HTTP client can just intercept everything and append an ID.



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

Reply via email to