amogh-jahagirdar commented on code in PR #10908: URL: https://github.com/apache/iceberg/pull/10908#discussion_r1717091505
########## open-api/src/testFixtures/java/org/apache/iceberg/rest/RESTCatalogServer.java: ########## @@ -0,0 +1,125 @@ +/* + * 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.iceberg.rest; + +import java.io.File; +import java.io.IOException; +import java.util.Map; +import org.apache.hadoop.conf.Configuration; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.CatalogUtil; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.jdbc.JdbcCatalog; +import org.apache.iceberg.util.PropertyUtil; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.component.LifeCycle; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class RESTCatalogServer implements LifeCycle.Listener { + private static final Logger LOG = LoggerFactory.getLogger(RESTCatalogServer.class); + + static final String REST_PORT = "rest.port"; + static final int REST_PORT_DEFAULT = 8181; + + private Server httpServer; + + RESTCatalogServer() {} + + static class CatalogContext { + private final Catalog catalog; + private final Map<String, String> configuration; + + CatalogContext(Catalog catalog, Map<String, String> configuration) { + this.catalog = catalog; + this.configuration = configuration; + } + + public Catalog getCatalog() { + return catalog; + } + + public Map<String, String> getConfiguration() { + return configuration; + } + } + + private CatalogContext initializeBackendCatalog() throws IOException { + // Translate environment variables to catalog properties + Map<String, String> catalogProperties = RCKUtils.environmentCatalogConfig(); + + // Fallback to a JDBCCatalog impl if one is not set + catalogProperties.putIfAbsent(CatalogProperties.CATALOG_IMPL, JdbcCatalog.class.getName()); + catalogProperties.putIfAbsent(CatalogProperties.URI, "jdbc:sqlite::memory:"); + catalogProperties.putIfAbsent("jdbc.schema-version", "V1"); + + // Configure a default location if one is not specified + String warehouseLocation = catalogProperties.get(CatalogProperties.WAREHOUSE_LOCATION); + + if (warehouseLocation == null) { + File tmp = java.nio.file.Files.createTempDirectory("iceberg_warehouse").toFile(); + tmp.deleteOnExit(); + warehouseLocation = tmp.toPath().resolve("iceberg_data").toFile().getAbsolutePath(); + catalogProperties.put(CatalogProperties.WAREHOUSE_LOCATION, warehouseLocation); + + LOG.info("No warehouse location set. Defaulting to temp location: {}", warehouseLocation); + } + + LOG.info("Creating catalog with properties: {}", catalogProperties); + return new CatalogContext( + CatalogUtil.buildIcebergCatalog("rest_backend", catalogProperties, new Configuration()), + catalogProperties); + } + + public void start(boolean join) throws Exception { + CatalogContext catalogContext = initializeBackendCatalog(); + + RESTCatalogAdapter adapter = new RESTServerCatalogAdapter(catalogContext); + RESTCatalogServlet servlet = new RESTCatalogServlet(adapter); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.NO_SESSIONS); + context.setContextPath("/"); + ServletHolder servletHolder = new ServletHolder(servlet); + servletHolder.setInitParameter("javax.ws.rs.Application", "ServiceListPublic"); + context.addServlet(servletHolder, "/*"); + context.setVirtualHosts(null); + context.insertHandler(new GzipHandler()); Review Comment: Could we double check if a lot of these context update calls are actually required? I believe these may have just been copied from [`TestRestViewCatalog`](https://github.com/apache/iceberg/blob/main/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java#L101) but from my local testing of `TestRestViewCatalog` it seems like we really only need to initialize ServletHolder and adding that to the context. setting the virtual hosts, and GZIP handler kind of stood out because it seemed like these should be defaults in the servlet context that don't need explicit overriding. The setting of virtual hosts, the insert handler and the init parameter don't seem to be needed for successful and meaningful tests? But do let me know if I'm wrong on this cc @nastra -- 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
