deniskuzZ commented on code in PR #6327:
URL: https://github.com/apache/hive/pull/6327#discussion_r2911125027
##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/standalone/StandaloneRESTCatalogServer.java:
##########
@@ -46,85 +42,50 @@
*
* <p>Multiple instances can run behind a Kubernetes Service for load
balancing.
*/
+@SpringBootApplication(exclude = DataSourceAutoConfiguration.class)
public class StandaloneRESTCatalogServer {
private static final Logger LOG =
LoggerFactory.getLogger(StandaloneRESTCatalogServer.class);
private final Configuration conf;
- private Server server;
+ private String restEndpoint;
private int port;
- public StandaloneRESTCatalogServer(Configuration conf) {
- this.conf = conf;
- }
-
/**
- * Starts the standalone REST Catalog server.
+ * Constructor that accepts Configuration.
+ * Standard Hive approach - caller controls Configuration creation.
*/
- public void start() {
+ public StandaloneRESTCatalogServer(Configuration conf) {
+ this.conf = conf;
+
// Validate required configuration
String thriftUris = MetastoreConf.getVar(conf, ConfVars.THRIFT_URIS);
if (thriftUris == null || thriftUris.isEmpty()) {
throw new IllegalArgumentException("metastore.thrift.uris must be
configured to connect to HMS");
}
- int servletPort = MetastoreConf.getIntVar(conf,
ConfVars.CATALOG_SERVLET_PORT);
- String servletPath = MetastoreConf.getVar(conf,
ConfVars.ICEBERG_CATALOG_SERVLET_PATH);
-
- if (servletPath == null || servletPath.isEmpty()) {
- servletPath = "iceberg"; // Default path
- MetastoreConf.setVar(conf, ConfVars.ICEBERG_CATALOG_SERVLET_PATH,
servletPath);
- }
-
- LOG.info("Starting Standalone REST Catalog Server");
+ LOG.info("Hadoop Configuration initialized");
LOG.info(" HMS Thrift URIs: {}", thriftUris);
- LOG.info(" Servlet Port: {}", servletPort);
- LOG.info(" Servlet Path: /{}", servletPath);
-
- // Create servlet using factory
- ServletServerBuilder.Descriptor catalogDescriptor =
HMSCatalogFactory.createServlet(conf);
- if (catalogDescriptor == null) {
- throw new IllegalStateException("Failed to create REST Catalog servlet.
" +
- "Check that metastore.catalog.servlet.port and
metastore.iceberg.catalog.servlet.path are configured.");
- }
-
- // Create health check servlet
- HealthCheckServlet healthServlet = new HealthCheckServlet();
-
- // Build and start server
- ServletServerBuilder builder = new ServletServerBuilder(conf);
- builder.addServlet(catalogDescriptor);
- builder.addServlet(servletPort, "health", healthServlet);
-
- server = builder.start(LOG);
- if (server == null || !server.isStarted()) {
- // Server failed to start - likely a port conflict
- throw new IllegalStateException(String.format(
- "Failed to start REST Catalog server on port %d. Port may already be
in use. ", servletPort));
+
+ if (LOG.isInfoEnabled()) {
+ LOG.info(" Warehouse: {}", MetastoreConf.getVar(conf,
ConfVars.WAREHOUSE));
}
-
- // Get actual port (may be auto-assigned)
- port = catalogDescriptor.getPort();
- LOG.info("Standalone REST Catalog Server started successfully on port {}",
port);
- LOG.info(" REST Catalog endpoint: http://localhost:{}/{}";, port,
servletPath);
- LOG.info(" Health check endpoint: http://localhost:{}/health";, port);
}
/**
- * Stops the server.
+ * Updates port and restEndpoint with the actual server port once the web
server has started.
+ * Handles RANDOM_PORT (tests) and server.port=0 where the real port differs
from config.
*/
- public void stop() {
- if (server != null && server.isStarted()) {
- try {
- LOG.info("Stopping Standalone REST Catalog Server");
- server.stop();
- server.join();
- LOG.info("Standalone REST Catalog Server stopped");
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- LOG.warn("Server stop interrupted", e);
- } catch (Exception e) {
- LOG.error("Error stopping server", e);
+ @EventListener
+ public void onWebServerInitialized(WebServerInitializedEvent event) {
+ int actualPort = event.getWebServer().getPort();
+ if (actualPort > 0) {
+ this.port = actualPort;
+ String servletPath = MetastoreConf.getVar(conf,
ConfVars.ICEBERG_CATALOG_SERVLET_PATH);
+ if (servletPath == null || servletPath.isEmpty()) {
+ servletPath = "iceberg";
}
+ this.restEndpoint = "http://localhost:"; + actualPort + "/" + servletPath;
Review Comment:
do we need to construct this manually of iceberg RestCatalog has utils for
that? if not, maybe
````
this.restEndpoint = UriComponentsBuilder
.scheme("http")
.host("localhost")
.port(actualPort)
.pathSegment(servletPath)
.toUriString();
````
should we support ssl?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
