Add support for environment context by ebyhr · Pull Request #3441 · apache/iceberg-python

ebyhr · 2026-05-30T00:53:56Z

Rationale for this change

Adds an EnvironmentContext utility class that provides the PyIceberg engine name and version for new snapshot summaries. Existing snapshots are unchanged.

Are these changes tested?

Yes. Tests cover metadata values, dictionary independence, snapshot writes, and property precedence.

Are there any user-facing changes?

Yes. New snapshot summaries include engine-name and engine-version, overriding same-named entries in snapshot_properties.

abnobdoss

This is great! This looks very helpful for debugging.

# Rationale for this change Allow users to create a view with fewer parameters. We can set an environment context (`engine-name` and `engine-version`) in `summary` field by default once #3441 is merged. ## Are these changes tested? Yes ## Are there any user-facing changes? No

kevinjqliu

Thanks for the PR! This will be very useful to users. I got a couple of nits

kevinjqliu · 2026-06-21T01:04:06Z

    timestamp_ms: int = Field(alias="timestamp-ms", default_factory=lambda: int(time.time() * 1000))
    """Timestamp when the version was created (ms from epoch)"""
-    summary: dict[str, str] = Field(default_factory=dict)
+    summary: dict[str, str] = Field(default_factory=lambda: EnvironmentContext.get())


this will add EnvironmentContext to all ViewVersion which might not be the desired behavior.

Similar to Table Metadata, can we only add EnvironmentContext in the write path?

kevinjqliu · 2026-06-21T01:09:49Z

        "total-files-size": str(file_size),
        "total-position-deletes": "0",
        "total-records": "3",
+        "engine-name": "pyiceberg",


Could we find a way to avoid repeating env context everywhere? Perhaps a small test helper like this would keep the exact assertions while localizing this new default behavior:

def with_environment_context(summary: dict[str, str]) -> dict[str, str]: return {**summary, **EnvironmentContext.get()} assert summaries[0] == with_environment_context({ "added-data-files": "3", "added-records": "5", ... })

kevinjqliu · 2026-06-21T01:13:12Z

+def test_default_value() -> None:
+    actual = EnvironmentContext.get()
+    assert len(actual) == 2
+    assert actual["engine-name"] == "pyiceberg"
+    assert re.match(r"^\d+\.\d+\.\d+", actual["engine-version"])


Suggested change

def test_default_value() -> None:

actual = EnvironmentContext.get()

assert len(actual) == 2

assert actual["engine-name"] == "pyiceberg"

assert re.match(r"^\d+\.\d+\.\d+", actual["engine-version"])

def test_default_value() -> None:

assert EnvironmentContext.get() == {

"engine-name": "pyiceberg",

"engine-version": __version__,

}

def test_get_returns_copy() -> None:

actual = EnvironmentContext.get()

actual["test-key"] = "test-value"

assert "test-key" not in EnvironmentContext.get()

EnvironmentContext might have been mutated, this is a better way to test that the returned value is a copy

+1 on this. The regex is really brittle and won't work for RC builds.

Ugh, I was missing the RC build.

I intentionally avoided using the same logic between environment_context.py and this test. I generally don't believe it's a good idea to write such tests.

kevinjqliu · 2026-06-21T01:13:41Z

+def test_put_and_remove() -> None:
+    EnvironmentContext.put("test-key", "test-value")
+    assert EnvironmentContext.get()["test-key"] == "test-value"
+
+    EnvironmentContext.remove("test-key")
+    assert "test-key" not in EnvironmentContext.get()


Suggested change

def test_put_and_remove() -> None:

EnvironmentContext.put("test-key", "test-value")

assert EnvironmentContext.get()["test-key"] == "test-value"

EnvironmentContext.remove("test-key")

assert "test-key" not in EnvironmentContext.get()

def test_put_and_remove() -> None:

try:

EnvironmentContext.put("test-key", "test-value")

assert EnvironmentContext.get()["test-key"] == "test-value"

assert EnvironmentContext.remove("test-key") == "test-value"

assert "test-key" not in EnvironmentContext.get()

finally:

EnvironmentContext.remove("test-key")

use try/finally to avoid mutating EnvironmentContext

kevinjqliu · 2026-06-21T01:17:00Z

+    for key, value in EnvironmentContext.get().items():
+        summary.__setitem__(key, value)


Suggested change

for key, value in EnvironmentContext.get().items():

summary.__setitem__(key, value)

for key, value in EnvironmentContext.get().items():

summary[key] = value

nit

kevinjqliu · 2026-06-21T01:18:55Z

+class EnvironmentContext:
+    _PROPERTIES: dict[str, str] = {
+        "engine-name": "pyiceberg",
+        "engine-version": version("pyiceberg"),


Suggested change

"engine-version": version("pyiceberg"),

"engine-version": __version__,

nit

rambleraptor · 2026-06-21T01:51:27Z

This looks great! My biggest note is to remove the version regexes, since they won't work on RCs.

github-actions · 2026-07-27T00:38:34Z

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

github-actions · 2026-08-27T01:35:19Z

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

Return fresh engine metadata, copy only engine fields into new snapshot summaries, and cover writes and property precedence without changing existing summary assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

kevinjqliu

LGTM!

kevinjqliu · 2026-09-13T22:41:01Z

+    if context := EnvironmentContext.get():
+        # Defensively select only engine fields so future context additions cannot overwrite snapshot metadata.
+        summary["engine-name"] = context["engine-name"]
+        summary["engine-version"] = context["engine-version"]


i think this is the best because we dont want to accidentally allow EnvironmentContext override other summary

kevinjqliu · 2026-09-13T22:41:45Z

+@pytest.fixture(autouse=True, scope="session")
+def _disable_environment_context() -> Generator[None, None, None]:


this allows us to not make a lot of changes to existing tests

ebyhr force-pushed the ebi/environment-context branch 5 times, most recently from 838ef11 to 07587a3 Compare May 30, 2026 04:22

ebyhr marked this pull request as ready for review June 1, 2026 03:20

abnobdoss approved these changes Jun 2, 2026

View reviewed changes

ebyhr mentioned this pull request Jun 5, 2026

Add defaults to ViewVersion fields #3458

Merged

ebyhr force-pushed the ebi/environment-context branch from 07587a3 to afe8bec Compare June 9, 2026 22:40

ebyhr mentioned this pull request Jun 9, 2026

Add more examples for pyiceberg view #3414

Merged

kevinjqliu reviewed Jun 21, 2026

View reviewed changes

ebyhr force-pushed the ebi/environment-context branch 2 times, most recently from 8f55791 to 61e5f42 Compare June 26, 2026 06:39

github-actions Bot added the stale label Jul 27, 2026

ebyhr force-pushed the ebi/environment-context branch from 61e5f42 to 507431b Compare July 27, 2026 01:03

github-actions Bot removed the stale label Jul 28, 2026

github-actions Bot added the stale label Aug 27, 2026

ebyhr added 2 commits August 27, 2026 10:44

Add support for environment context

4390268

fixup! Add support for environment context

abaf725

ebyhr force-pushed the ebi/environment-context branch from 507431b to abaf725 Compare August 27, 2026 01:44

github-actions Bot removed the stale label Aug 28, 2026

kevinjqliu force-pushed the ebi/environment-context branch from f932d18 to 93ba3f1 Compare September 13, 2026 22:39

kevinjqliu approved these changes Sep 13, 2026

View reviewed changes

kevinjqliu enabled auto-merge September 13, 2026 22:45

kevinjqliu added this pull request to the merge queue Sep 13, 2026

Merged via the queue into apache:main with commit 0e4ac51 Sep 13, 2026
21 checks passed

		for key, value in EnvironmentContext.get().items():
		summary.__setitem__(key, value)

	"engine-version": version("pyiceberg"),
	"engine-version": __version__,

		@pytest.fixture(autouse=True, scope="session")
		def _disable_environment_context() -> Generator[None, None, None]:

Uh oh!

Conversation

ebyhr commented May 30, 2026 • edited by kevinjqliu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

Are these changes tested?

Are there any user-facing changes?

Uh oh!

abnobdoss left a comment

Choose a reason for hiding this comment

Uh oh!

kevinjqliu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ebyhr Jun 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rambleraptor commented Jun 21, 2026

Uh oh!

github-actions Bot commented Jul 27, 2026

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

kevinjqliu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ebyhr commented May 30, 2026 •

edited by kevinjqliu

Loading

ebyhr Jun 21, 2026 •

edited

Loading