You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "nandorKollar (via GitHub)" <gi...@apache.org> on 2026/05/04 06:45:59 UTC
[PR] Remove custom docker image build from HMS integration test [polaris]
nandorKollar opened a new pull request, #4361:
URL: https://github.com/apache/polaris/pull/4361
<!--
๐ Describe what changes you're proposing, especially breaking or user-facing changes.
๐ See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
-->
For the HMS federation integration tests, we create our own Docker images from the Hive image. This seems unnecessary, as the reason for it was that the AWS libs were missing from the classpath. However, it seems that the required jars are there in the container (in `/opt/hadoop/share/hadoop/tools/lib/` folder), we just need to tweak the classpath accordingly: `HADOOP_CLASSPATH=$HADOOP_CLASSPATH:/opt/hadoop/share/hadoop/tools/lib/*`
## Checklist
- [ ] ๐ก๏ธ Don't disclose security issues! (contact security@apache.org)
- [ ] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes #
- [ ] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] ๐ก Added comments for complex logic
- [ ] ๐งพ Updated `CHANGELOG.md` (if needed)
- [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3184121687
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/HiveCatalogFederationIntegrationTest.java:
##########
@@ -107,6 +107,8 @@ public abstract class HiveCatalogFederationIntegrationTest {
private static final String PRINCIPAL_ROLE_NAME = "test-hive-federation-user-role";
private static final String DIRECT_CATALOG_NAME = "hms_direct";
+ private static final String HMS_VERSION = "4.0.1";
Review Comment:
Agreed: you should keep the docker file with just a FROM statement.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3179799159
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/HiveCatalogFederationIntegrationTest.java:
##########
@@ -107,6 +107,8 @@ public abstract class HiveCatalogFederationIntegrationTest {
private static final String PRINCIPAL_ROLE_NAME = "test-hive-federation-user-role";
private static final String DIRECT_CATALOG_NAME = "hms_direct";
+ private static final String HMS_VERSION = "4.0.1";
Review Comment:
We could consider parametrise this test with the HMS version, and test against HMS 3.x and 4.x too. I expect one would use this federation option against HMS 3.x more often, as for HMS 4 the REST catalog federation should also work, and should be preferred choice IMHO.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4361:
URL: https://github.com/apache/polaris/pull/4361#issuecomment-4408547655
Thanks, @nandorKollar !
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3183217046
##########
tools/hms-testcontainer/src/main/java/org/apache/polaris/test/hms/HmsContainer.java:
##########
@@ -56,24 +47,24 @@ public final class HmsContainer extends GenericContainer<HmsContainer> implement
private String s3aSecretKey;
@SuppressWarnings("resource")
- public HmsContainer(String image) {
- super(buildImage(image));
+ public HmsContainer(String version) {
+ super("docker.io/apache/hive:" + version);
withLogConsumer(new Slf4jLogConsumer(LoggerFactory.getLogger(HmsContainer.class)));
addExposedPort(THRIFT_PORT);
// The apache/hive image dispatches on this env var:
// metastore -> launches HiveMetaStore (Thrift on 9083)
// hiveserver2 -> launches HiveServer2
withEnv("SERVICE_NAME", "metastore");
+ // The apache/hive images do not ship hadoop-aws or the AWS SDK in /opt/hive/lib,
+ // but in /opt/hadoop/share/hadoop/tools/lib/, which is not added to HADOOP_CLASSPATH by
+ // default.
+ withEnv("HADOOP_CLASSPATH", "$HADOOP_CLASSPATH:/opt/hadoop/share/hadoop/tools/lib/*");
Review Comment:
Minor: do we need to add `$HADOOP_CLASSPATH:`? I think `"HADOOP_CLASSPATH"` is just expanded once, no more further expanding.
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/HiveCatalogFederationIntegrationTest.java:
##########
@@ -107,6 +107,8 @@ public abstract class HiveCatalogFederationIntegrationTest {
private static final String PRINCIPAL_ROLE_NAME = "test-hive-federation-user-role";
private static final String DIRECT_CATALOG_NAME = "hms_direct";
+ private static final String HMS_VERSION = "4.0.1";
Review Comment:
HMS version was Renovate-managed via the Dockerfile `FROM` line before. now that it's a hardcoded java constant, Renovate won't pick it up.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3196503256
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/HiveCatalogFederationIntegrationTest.java:
##########
@@ -107,6 +107,8 @@ public abstract class HiveCatalogFederationIntegrationTest {
private static final String PRINCIPAL_ROLE_NAME = "test-hive-federation-user-role";
private static final String DIRECT_CATALOG_NAME = "hms_direct";
+ private static final String HMS_VERSION = "4.0.1";
Review Comment:
How about an alternative way like adding a Renovate regex rule for the HMS_VERSION constant (or a // renovate: marker comment) so image version bumps don't silently go stale?
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3205337616
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/HiveCatalogFederationIntegrationTest.java:
##########
@@ -107,6 +107,8 @@ public abstract class HiveCatalogFederationIntegrationTest {
private static final String PRINCIPAL_ROLE_NAME = "test-hive-federation-user-role";
private static final String DIRECT_CATALOG_NAME = "hms_direct";
+ private static final String HMS_VERSION = "4.0.1";
Review Comment:
`ContainerSpecHelper` was created to facilitate loading version values from Dockerfile. This is to allow Renovate to upgrade versions in Dockerfile.
If we need to test HMS 3.x and 4.x I think the best way would be to manage two docker files, e.g. `Dockerfile-hms3-version` and `Dockerfile-hms4-version` and configure Renovate to avoid changing the major version number automatically. WDYT?
Cf. https://github.com/apache/polaris/blob/9fe8fc97b32a0380fc97bb9e1ff59e948dd5bac7/tools/rustfs-testcontainer/src/main/java/org/apache/polaris/test/rustfs/RustfsContainer.java#L136
Cf. https://github.com/apache/polaris/blob/1d8dc001026e0ce4498c6a3aa3b831d1109a02db/tools/rustfs-testcontainer/src/main/resources/org/apache/polaris/test/rustfs/Dockerfile-rustfs-version#L22
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3205649871
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/HiveCatalogFederationIntegrationTest.java:
##########
@@ -107,6 +107,8 @@ public abstract class HiveCatalogFederationIntegrationTest {
private static final String PRINCIPAL_ROLE_NAME = "test-hive-federation-user-role";
private static final String DIRECT_CATALOG_NAME = "hms_direct";
+ private static final String HMS_VERSION = "4.0.1";
Review Comment:
Thanks @dimas-b , sounds good! It seems we already follow a similar pattern with Mongodb images, where we keep a one-liner Docker file.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on PR #4361:
URL: https://github.com/apache/polaris/pull/4361#issuecomment-4368830736
@adutra let me know what do you think, we could spare our custom Hive image builds in the integration tests.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3185544076
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/HiveCatalogFederationIntegrationTest.java:
##########
@@ -107,6 +107,8 @@ public abstract class HiveCatalogFederationIntegrationTest {
private static final String PRINCIPAL_ROLE_NAME = "test-hive-federation-user-role";
private static final String DIRECT_CATALOG_NAME = "hms_direct";
+ private static final String HMS_VERSION = "4.0.1";
Review Comment:
Oh, okay. So we will have to maintain this version here AND in the dockerfile then? It would be better, if we could completely remove the dockerfile, and maintain the HMS version in a single place. What do you think btw, overall is it better this way, or we should keep the original approach, and have our own Docker images? I think the disadvantage of keeping the original approach is: longer build time and we need to maintain the versions in our dockerfile, make sure that the aws dependency version match with the expected etc.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3210317375
##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/HiveCatalogFederationIntegrationTest.java:
##########
@@ -107,6 +107,8 @@ public abstract class HiveCatalogFederationIntegrationTest {
private static final String PRINCIPAL_ROLE_NAME = "test-hive-federation-user-role";
private static final String DIRECT_CATALOG_NAME = "hms_direct";
+ private static final String HMS_VERSION = "4.0.1";
Review Comment:
Yes, there are a few examples of that pattern :+1:
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #4361:
URL: https://github.com/apache/polaris/pull/4361
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove custom docker image build from HMS integration test [polaris]
Posted by "nandorKollar (via GitHub)" <gi...@apache.org>.
nandorKollar commented on code in PR #4361:
URL: https://github.com/apache/polaris/pull/4361#discussion_r3188909281
##########
tools/hms-testcontainer/src/main/java/org/apache/polaris/test/hms/HmsContainer.java:
##########
@@ -56,24 +47,24 @@ public final class HmsContainer extends GenericContainer<HmsContainer> implement
private String s3aSecretKey;
@SuppressWarnings("resource")
- public HmsContainer(String image) {
- super(buildImage(image));
+ public HmsContainer(String version) {
+ super("docker.io/apache/hive:" + version);
withLogConsumer(new Slf4jLogConsumer(LoggerFactory.getLogger(HmsContainer.class)));
addExposedPort(THRIFT_PORT);
// The apache/hive image dispatches on this env var:
// metastore -> launches HiveMetaStore (Thrift on 9083)
// hiveserver2 -> launches HiveServer2
withEnv("SERVICE_NAME", "metastore");
+ // The apache/hive images do not ship hadoop-aws or the AWS SDK in /opt/hive/lib,
+ // but in /opt/hadoop/share/hadoop/tools/lib/, which is not added to HADOOP_CLASSPATH by
+ // default.
+ withEnv("HADOOP_CLASSPATH", "$HADOOP_CLASSPATH:/opt/hadoop/share/hadoop/tools/lib/*");
Review Comment:
I added `$HADOOP_CLASSPATH:` just in case someone somehow already uses it within the Hive container.
--
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-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org