Skip to content

Support contextmanagers as assets#246

Open
sneakers-the-rat wants to merge 7 commits into
mainfrom
issue-110
Open

Support contextmanagers as assets#246
sneakers-the-rat wants to merge 7 commits into
mainfrom
issue-110

Conversation

@sneakers-the-rat

@sneakers-the-rat sneakers-the-rat commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Part of #110 but doesn't close it.

This is the first half of the issue, allowing contextmanagers as assets, but it does not address the main point of the issue, which is about providing some kind of "middleware" that can wrap around nodes for repeated operations across nodes.

This PR just makes it possible to use contextmanagers as assets, simple as.

however during testing, discovered a few bugs in how assets were handling both in the base runner and the async runner:

  • The base runner was not passing edges to the state when initializing node-scoped assets, so all node-scoped assets were initialized every time (bad)
  • The logic in state was flawed to allow that to happen - we should only initialize node-scoped assets when they are directly depended on by the node.
  • async runner double-initialized node-scoped assets, and didn't deinit process-scoped assets.

These were discovered here because the test for the contextmanager included init/deinit counts, which were off in the flawed code. the tests here do not test global counts of init/deinit, just per asset instance, so further testing to constrain the behavior would be good here.


📚 Documentation preview 📚: https://noob--246.org.readthedocs.build/en/246/

@coveralls

coveralls commented Jul 7, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 84.711% (+0.1%) from 84.589% — issue-110 into main

@sneakers-the-rat sneakers-the-rat marked this pull request as ready for review July 7, 2026 19:31
@codspeed-hq

codspeed-hq Bot commented Jul 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 8 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing issue-110 (e8449d9) with main (5c236d7)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

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.

2 participants