Skip to content

feat(table): allow explicit runtime for static tables#2501

Open
officialasishkumar wants to merge 1 commit into
apache:mainfrom
officialasishkumar:feat-static-table-runtime
Open

feat(table): allow explicit runtime for static tables#2501
officialasishkumar wants to merge 1 commit into
apache:mainfrom
officialasishkumar:feat-static-table-runtime

Conversation

@officialasishkumar
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

What changes are included in this PR?

  • Add StaticTable::from_metadata_with_runtime for constructing static tables with an explicit Iceberg runtime.
  • Add StaticTable::from_metadata_file_with_runtime for metadata-file construction with an explicit runtime.
  • Preserve the existing constructors by delegating them through the new runtime-aware APIs.

Are these changes tested?

  • CARGO_TARGET_DIR=/tmp/iceberg-rust-target cargo test --locked -p iceberg with_runtime -- --nocapture
  • cargo fmt --all -- --check
  • git diff --check

Add StaticTable constructors that accept an iceberg Runtime directly while preserving the existing current-runtime based constructors. Cover both metadata-file construction and construction from in-memory metadata outside a Tokio context.
Copy link
Copy Markdown
Collaborator

@CTTY CTTY left a comment

Choose a reason for hiding this comment

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

Hi @officialasishkumar , thanks for picking this up!

I think we should change the API and ask for an explicit runtime because runtime is always mandatory, not adding a new API. If users want to use the current runtime, they can call Runtime::try_current()

@@ -302,12 +302,23 @@ impl StaticTable {
metadata: TableMetadata,
table_ident: TableIdent,
file_io: FileIO,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should ask for an explicit runtime here rather than falling back to the current runtime

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.

Support using custom runtime with StaticTable

2 participants