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

Fix typing now that code is tuple of frame(s) #7778

Merged
merged 2 commits into from Apr 17, 2023

Conversation

ntabris
Copy link
Contributor

@ntabris ntabris commented Apr 14, 2023

Return type of _get_computation_code was changed from str to tuple[str] in #7656.

Whatever that returns gets sent to scheduler.update_graph:

"op": "update-graph",
"graph_header": header,
"graph_frames": frames,
"keys": list(map(stringify, keys)),
"internal_priority": internal_priority,
"submitting_task": getattr(thread_state, "key", None),
"fifo_timeout": fifo_timeout,
"actors": actors,
"code": self._get_computation_code(
nframes=dask.config.get(
"distributed.diagnostics.computations.nframes"
)
),

So type annotation on code argument is now incorrect. This fixes that.

@ntabris ntabris requested a review from fjetter as a code owner April 14, 2023 21:42
@github-actions
Copy link
Contributor

Unit Test Results

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

       26 files  ±0         26 suites  ±0   13h 44m 35s ⏱️ - 28m 39s
  3 551 tests ±0    3 443 ✔️ +4     106 💤 ±0  2  - 4 
44 923 runs  ±0  42 680 ✔️ +3  2 241 💤 +2  2  - 5 

For more details on these failures, see this check.

Results for commit b03a631. ± Comparison against base commit 515dffe.

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @ntabris! Any idea why this is currently passing our mypy check?

@ntabris
Copy link
Contributor Author

ntabris commented Apr 17, 2023

Any idea why this is currently passing our mypy check?

I'm assuming typing doesn't get checked across RPC boundary, i.e., send_to_scheduler with "op" set.

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Hmm that's a shame. Oh well, I agree that this is correct now. Thanks for fixing!

@jrbourbeau jrbourbeau merged commit 5c0a412 into dask:main Apr 17, 2023
30 of 33 checks passed
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.

None yet

2 participants