Skip to content

Extract guice bindings for custom Iceberg catalog#14192

Merged
findepi merged 3 commits intotrinodb:masterfrom
findinpath:iceberg-catalog-module-refactoring
Sep 20, 2022
Merged

Extract guice bindings for custom Iceberg catalog#14192
findepi merged 3 commits intotrinodb:masterfrom
findinpath:iceberg-catalog-module-refactoring

Conversation

@findinpath
Copy link
Copy Markdown
Contributor

Description

Allow tests to use a fully customized Iceberg catalog
that is not necessarily backed by a FileHiveMetastore instance.

Non-technical explanation

Test infrastructure change

Release notes

(x) This is not user-visible and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Sep 19, 2022
@findinpath findinpath self-assigned this Sep 19, 2022
@findinpath findinpath added the no-release-notes This pull request does not require release notes entry label Sep 19, 2022
Copy link
Copy Markdown
Member

@alexjo2144 alexjo2144 left a comment

Choose a reason for hiding this comment

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

Looks good to me. It's nice to be able to move some of that test only code out of IcebergCatalogModule.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it doesn't need to do this IMO

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

DecoratedHiveMetastoreModule creates a SharedHiveMetastoreCache which, depending on the CachingHiveMetastoreConfig configuration values may be enabled.

MetastoreValidator is there to specifically invalidate such a behaviour.

I think it makes sense to keep it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In tests we don't need to validate that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

but fine, we can keep it

Allow tests to use a fully customized Iceberg catalog
that is not necessarily backed by a `FileHiveMetastore` instance.
@findinpath findinpath force-pushed the iceberg-catalog-module-refactoring branch from 1e8b5a3 to 8ecbd5a Compare September 20, 2022 07:29
@findinpath findinpath requested a review from findepi September 20, 2022 07:32
@findepi findepi merged commit 5ac5fdb into trinodb:master Sep 20, 2022
@github-actions github-actions bot added this to the 397 milestone Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

3 participants