Skip to content

Conversation

@binarycat0
Copy link
Contributor

@binarycat0 binarycat0 commented Aug 18, 2025

Updates S3Mock testcontainer dependency from 3.11.0 to 4.7.0 and refactors usage into a centralized wrapper class in runtime/test-common.

Changes

  • Upgraded S3Mock testcontainer to 4.7.0
  • Created S3Mock wrapper class for consistent configuration
  • Consolidated S3 config properties generation
  • Updated integration tests to use new wrapper

No functional changes to test behavior.

@binarycat0 binarycat0 changed the title chore(deps): bump s3mock tag version from 3.11.0 to 4.7.0 chore(deps): bump s3mock from 3.11.0 to 4.7.0 Aug 18, 2025
Copy link
Member

@snazy snazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one is good as it is!

One suggestion to remove some dummy constants and one nit, but that's all.

@binarycat0 binarycat0 force-pushed the chore/bump-s3mock-from-3.11.0-to-4.7.0 branch from 305e0c9 to eb04048 Compare August 18, 2025 17:38
Updates S3Mock testcontainer dependency from 3.11.0 to 4.7.0 and refactors usage into a centralized wrapper class in runtime/test-common.
Changes

    Upgraded S3Mock testcontainer to 4.7.0
    Created S3Mock wrapper class for consistent configuration
    Consolidated S3 config properties generation
    Updated integration tests to use new wrapper

No functional changes to test behavior.
@binarycat0 binarycat0 force-pushed the chore/bump-s3mock-from-3.11.0-to-4.7.0 branch from 039779e to d2c60b3 Compare August 18, 2025 19:34
@binarycat0 binarycat0 marked this pull request as ready for review August 18, 2025 19:40
Copy link
Contributor

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM 👍 Thanks @binarycat0 !

Let's wait for @snazy and @pingtimeout do have another review round since they commented previously.

"table-default.s3.access-key-id", DEFAULT_ACCESS_KEY,
"table-default.s3.secret-access-key", DEFAULT_SECRET_KEY,
"s3.endpoint", endpoint,
"s3.path-style-access", "true",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

side note: why not use MinIO now that it is supported? It looks like from the Polaris POV using S3Mock is mostly the same as using any S3-compatible storage. The advantage of MinIO is that it is real (meaning not a mock).

This is just a tangential comment for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the comment! This is a good observation, looks like an idea for the next PR! The current PR was aimed at fixing the environment to unblock further steps in studying the project.

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Aug 18, 2025
Copy link
Member

@snazy snazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@snazy
Copy link
Member

snazy commented Aug 19, 2025

If @pingtimeout is happy, I think we can merge this one, as it's rather only a version bump & alignment and contains no functional change.

@pingtimeout pingtimeout merged commit 850792c into apache:main Aug 19, 2025
12 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Aug 19, 2025
bmlyr pushed a commit to bmlyr/polaris that referenced this pull request Aug 19, 2025
Updates S3Mock testcontainer dependency from 3.11.0 to 4.7.0 and refactors usage into a centralized wrapper class in runtime/test-common.

Changes

    Upgraded S3Mock testcontainer to 4.7.0
    Created S3Mock wrapper class for consistent configuration
    Consolidated S3 config properties generation
    Updated integration tests to use new wrapper

No functional changes to test behavior.
@binarycat0 binarycat0 deleted the chore/bump-s3mock-from-3.11.0-to-4.7.0 branch September 5, 2025 17:46
snazy added a commit to snazy/polaris that referenced this pull request Nov 20, 2025
* Refactor Authenticator and PolarisPrincipal (apache#2307)

The main goal of this change is to facilitate future integration of federated principals:

- `AuthenticatedPolarisPrincipal` becomes an interface `PolarisPrincipal`, as the original class leaks implementation details (references to `PrincipalEntity` and thus to the storage layer). The new interface does not reference the storage layer. This is one step further towards easy pluggability of authentication in Polaris.

- The `Authenticator.authenticate()` method does not return an `Optional` anymore, as this was ambiguous (returning `Optional.empty()` vs throwing `NotAuthorizedException`).

- Also the `Authenticator` interface is not generic anymore. This was an artifact of times when there were two kinds of `Authenticators` in Polaris (one for internal auth, the other for external) and is not necessary anymore.

* Add PolarisDiagnostics field to TransactionalMetaStoreManagerImpl (apache#2361)

the ultimate goal is removing the PolarisCallContext parameter from every
PolarisMetaStoreManager interface method, so we make steps towards reducing
its usage first.

* Support HMS Federation (apache#2355)

Supports federating to HiveCatalog using the Iceberg REST library. 
All hive dependencies are added in an independent module, i.e., `polaris-extensions-federation-hive` and can be removed/converted to a compile time flag if necessary. 
Similar to HadoopCatalog, HMS federation support is currently restricted to `IMPLICIT` auth. The underlying authentication can be any form that Hive supports, however Polaris will not store and manage any of these credentials. Again, similar to HadoopCatalog, this version supports federating to a single Hive instance. 

This PR relies on Polaris discovering the `hive-site.xml` file to get the configuration options from the classpath (including `HADOOP_CONF_DIR`).

The spec change has been discussed in the [dev mailing list](https://lists.apache.org/thread/5qktjv6rzd8pghcl6f4oohko798o2p2g), followed by a discussion in the Polaris community sync on Aug 7, 2025. 

Testing: 
Modified the regression test to locally test that Hive federation works as expected. The next step would be to add a regression test once the change is baked into the Polaris docker image (for CI builds).


This PR primarily builds on apache#1305 and apache#1466. Thank you @dennishuo and @eric-maynard for helping out with this!

* Add PolarisDiagnostics field to TransactionWorkspaceMetaStoreManager (apache#2359)

the ultimate goal is removing the `PolarisCallContext` parameter from every
`PolarisMetaStoreManager` interface method, so we make steps towards
reducing its usage first.

* Rat-ignore user-settings for hugo-run-in-docker (apache#2376)

* Modularize generic table federation (apache#2379)

In apache#2369 Iceberg table federation was refactored around the new `IcebergRESTExternalCatalogFactory` type based on discussion in the community sync. This has unblocked the ability to federate to more non-Iceberg catalogs, such as in apache#2355. This PR refactors generic table federation to go through the same mechanism. After this, we can go through and implement generic table federation for the existing `IcebergRESTExternalCatalogFactory` implementations.

* Update community meeting dates (apache#2382)

* Reduce getRealmConfig calls (apache#2337)

Classes with a `CallContext` field should call `getRealmConfig` once and store it as a field as well.
The idea is that long term we would want to stop relying on the `CallContext` itself but instead inject its individual items. Thus we also add `RealmConfig` to `TestServices`.

* Python client: make S3 role-ARN optional and add missing endpoint-internal property (apache#2339)

* fix(deps): update dependency io.prometheus:prometheus-metrics-exporter-servlet-jakarta to v1.4.1 (apache#2377)

* chore(deps): bump s3mock from 3.11.0 to 4.7.0 (apache#2375)

Updates S3Mock testcontainer dependency from 3.11.0 to 4.7.0 and refactors usage into a centralized wrapper class in runtime/test-common.

Changes

    Upgraded S3Mock testcontainer to 4.7.0
    Created S3Mock wrapper class for consistent configuration
    Consolidated S3 config properties generation
    Updated integration tests to use new wrapper

No functional changes to test behavior.

* Nit: extract getResolvedCatalogEntity method in IcebergCatalogHandler (apache#2387)

* Nit: remove transitive dependencies from runtime/server/build.gradle.kts (apache#2385)

* Nit: add methods isExternal and isStaticFacade to CatalogEntity (apache#2386)

* Minor refactor of integration test classes (apache#2384)

This change promotes `CatalogConfig` and `RestCatalogConfig` to top-level, public annotations and introduces a few "hooks" in `PolarisRestCatalogIntegrationBase` that can be overridden by subclasses.

This change is a preparatory work for apache#2280 (S3 remote signing).

* Remove BaseMetaStoreManager.serializeProperties (apache#2374)

similar to 7af85be we should prefer
the existing helper methods on the entity instead

* fix: minor corrections of documentation (apache#2397)

- fixed dead link to catalog definition in Iceberg docs on Entities page
- removed single quotes from credential parameter in the cmdline example for connecting a local spark-sql: env variables need to be resolved in cmdline, they will not be resolved by spark-sql itself.

* chore(deps): update azure/setup-helm action to v4.3.1 (apache#2402)

* Add 1.0.1 release to the website (apache#2400)

* Add PolarisDiagnostics field to AbstractTransactionalPersistence (apache#2372)

The ultimate goal is removing the `PolarisCallContext` parameter from every `PolarisMetaStoreManager` interface method, so we make steps towards reducing its usage first.

* NoSQL: javadoc nit

* Last merged commit fcd4777

---------

Co-authored-by: Alexandre Dutra <adutra@apache.org>
Co-authored-by: Christopher Lambert <xn137@gmx.de>
Co-authored-by: Pooja Nilangekar <poojan@umd.edu>
Co-authored-by: Eric Maynard <eric.maynard+oss@snowflake.com>
Co-authored-by: JB Onofré <jbonofre@apache.org>
Co-authored-by: Mend Renovate <bot@renovateapp.com>
Co-authored-by: Artur Rakhmatulin <from_github@binaryc.at>
Co-authored-by: olsoloviov <40199597+olsoloviov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants