Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure inproc properly emulates serialization protocol #8622

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

fjetter
Copy link
Member

@fjetter fjetter commented Apr 17, 2024

Ran into this again in #8612. Quite annoyingly, the InProc comm is not actually serializing stuff but is just emulating the behavior of the serialization protocol. This is nice because it should make our tests faster but it doesn't reflect reality since the ToPickle functionality is actually implemented on the serialization side, i.e. if we never serialize, the object stays untouched.

I could see how this fix also slows down the testsuite but I don't think we should just ignore this.

@@ -4676,9 +4676,6 @@ async def update_graph(
annotations: dict | None = None,
stimulus_id: str | None = None,
) -> None:
# FIXME: Apparently empty dicts arrive as a ToPickle object
Copy link
Member Author

Choose a reason for hiding this comment

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

My initial diagnose was wrong. This has nothing to do with empty dicts

Copy link
Contributor

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

    29 files  ±0      29 suites  ±0   11h 11m 46s ⏱️ + 4m 47s
 4 065 tests ±0   3 950 ✅  - 1    109 💤 ±0  6 ❌ +1 
55 026 runs  ±0  52 611 ✅  - 1  2 407 💤  - 1  8 ❌ +2 

For more details on these failures, see this check.

Results for commit b1d7d43. ± Comparison against base commit 3f13a2d.

@fjetter fjetter merged commit f621c65 into dask:main Apr 18, 2024
26 of 35 checks passed
@fjetter fjetter deleted the fix_inproc_deserialization branch June 4, 2024 13:42
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.

1 participant