Skip to content

Conversation

@desertaxle
Copy link
Collaborator

@desertaxle desertaxle commented Oct 24, 2025

Summary

Enables dependency injection for both synchronous and asynchronous code, making it easier to:

  • Write pure computation dependencies without async boilerplate
  • Integrate with existing synchronous libraries
  • Mix sync and async dependencies in the same task

Important: Sync dependencies should NOT include blocking I/O. Use async dependencies for any I/O operations.

Changes

  • Extended type support: Depends() now accepts sync functions, sync context managers (@contextmanager), async functions, and async context managers (@asynccontextmanager)
  • Automatic detection: The system automatically detects the return type and handles it appropriately
  • Proper cleanup: Sync context managers clean up correctly even when tasks fail
  • Dependency caching: Works seamlessly across both sync and async dependencies

Examples

Sync dependency - pure computation

# ✅ Good: In-memory config, no I/O
def get_config() -> dict:
    return {"api_url": "https://api.example.com", "timeout": 30}

# ✅ Good: Pure computation
def build_query_params(
    user_id: int = TaskArgument(),
    config: dict = Depends(get_config)
) -> dict:
    return {"user_id": user_id, "timeout": config["timeout"]}

async def my_task(params: dict = Depends(build_query_params)) -> None:
    # Params computed without blocking
    await make_api_call(params)

Async dependency - I/O operations

# ✅ Good: Network I/O uses async
async def fetch_user_data(user_id: int = TaskArgument()) -> dict:
    return await api_client.get(f"/users/{user_id}")

async def my_task(user: dict = Depends(fetch_user_data)) -> None:
    await process_user(user)

What NOT to do

# ❌ Bad: Blocking I/O in sync dependency
def load_config_from_file() -> dict:
    with open("config.json") as f:  # Blocks event loop!
        return json.load(f)

# ✅ Good: Use async for file I/O instead
async def load_config_from_file() -> dict:
    async with aiofiles.open("config.json") as f:
        return json.loads(await f.read())

Use Cases

Best for sync dependencies:

  • Configuration merging and transformations
  • Data structure manipulations
  • Pure computations
  • In-memory cache lookups

Use async dependencies for:

  • Database queries
  • Network requests
  • File I/O
  • Any operation requiring await

Testing

  • Added 8 comprehensive tests in test_dependencies.py covering all sync scenarios
  • Added 6 tests in test_fundamentals.py to document the feature
  • All 14 new tests passing ✅
  • All existing tests continue to pass
  • 100% backward compatible

🤖 Generated with Claude Code

Enable dependency injection for both sync and async code to simplify
testing and allow integration with existing synchronous libraries.
Dependencies now support:
- Sync functions returning values
- Sync context managers (@contextmanager)
- Async functions (existing)
- Async context managers (existing)

All dependency types work seamlessly together with automatic detection
and proper resource cleanup.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@claude
Copy link
Contributor

claude bot commented Oct 24, 2025

Claude finished @desertaxle's task —— View job


Code Review: Add support for sync functions and context managers in Depends

Summary

This PR successfully extends the dependency injection system to support synchronous functions and context managers alongside the existing async support. The implementation is clean, well-tested, and properly documented. The feature maintains 100% backward compatibility while enabling more natural and efficient code patterns.

Strengths

  1. Elegant Implementation (src/docket/dependencies.py:484-500): The type detection and handling logic is clear and comprehensive:

    • Properly detects and handles async context managers
    • Correctly uses stack.enter_context() for sync context managers (non-blocking)
    • Handles both coroutines and awaitables
    • Falls through to direct value return for sync functions
  2. Comprehensive Test Coverage: Added 14 new tests covering all scenarios:

    • Basic sync function dependencies
    • Sync context manager with proper cleanup
    • Mixed sync/async dependencies
    • Nested dependency resolution
    • Exception handling and cleanup
    • Dependency caching behavior
  3. Clear Documentation: Excellent updates to docs/dependencies.md with:

    • Clear warnings about avoiding blocking I/O
    • Good vs bad examples
    • Practical use cases for each pattern

Performance Considerations

No performance concerns identified

  • Sync dependencies execute directly without async overhead
  • Context managers use proper stack management
  • Dependency caching prevents redundant calculations
  • The implementation correctly avoids blocking the event loop

Security Review

No security issues found

  • No new attack vectors introduced
  • Context manager cleanup properly ensures resource cleanup even on exceptions
  • No changes to authentication/authorization flows

Code Quality

The code follows best practices:

  • Type hints are comprehensive and accurate
  • The implementation reuses existing patterns from the codebase
  • Error handling is consistent with the rest of the project
  • Docstrings are thorough with helpful examples

Minor Suggestions for Future Improvements

  1. Runtime Warning for Blocking I/O (optional enhancement):
    Consider adding a debug mode that could detect and warn about potentially blocking operations in sync dependencies (e.g., file I/O, network calls). This could help developers catch misuse early.

  2. Documentation Enhancement (optional):
    The examples could benefit from showing a concrete FastAPI background tasks migration example since that's mentioned in the summary.

Edge Cases Handled Well

✅ Sync context manager cleanup on exception (test_sync_context_manager_cleanup_on_exception)
✅ Dependency caching across sync and async (test_sync_dependency_caching)
✅ Complex nesting patterns (test_mixed_nested_dependencies)
✅ Access to Docket context from sync deps (test_sync_dependency_with_docket_context)

Test Coverage

The coverage report confirms 100% coverage maintained with all new lines covered. The tests are well-structured and test both the happy path and error conditions.

Verdict: Approved

This is a well-crafted PR that adds valuable functionality without compromising the existing codebase. The sync support makes the dependency system more intuitive for simple cases while maintaining the async capabilities for I/O operations. The clear documentation about when to use sync vs async will help developers make the right choices.

Great work on this feature! The implementation is solid, the tests are comprehensive, and the documentation clearly guides users on proper usage.

@github-actions
Copy link

github-actions bot commented Oct 24, 2025

📚 Documentation has been built for this PR!

You can download the documentation directly here:
https://github.com/chrisguidry/docket/actions/runs/18789552879/artifacts/4366592341

@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (36f92a8) to head (e848f53).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              main      #172    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           34        34            
  Lines         4834      5047   +213     
  Branches       265       267     +2     
==========================================
+ Hits          4834      5047   +213     
Flag Coverage Δ
python-3.12 100.00% <100.00%> (ø)
python-3.13 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/docket/dependencies.py 100.00% <100.00%> (ø)
tests/test_dependencies.py 100.00% <100.00%> (ø)
tests/test_fundamentals.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

desertaxle and others added 2 commits October 24, 2025 13:02
Replace examples showing blocking operations (file I/O, locks) with
non-blocking patterns:
- Pure computations (config merging, param building)
- In-memory operations (dict/set access)
- Quick transformations

Add clear guidance that sync dependencies should NEVER include
blocking I/O operations. All I/O must use async dependencies.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@desertaxle desertaxle changed the title Add support for sync functions and context managers in Depends Add support for sync functions and context managers in Depends Oct 24, 2025
@desertaxle desertaxle marked this pull request as ready for review October 24, 2025 18:16
Copy link
Owner

@chrisguidry chrisguidry left a comment

Choose a reason for hiding this comment

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

Beautiful!

@chrisguidry chrisguidry merged commit ed8756c into main Oct 24, 2025
13 checks passed
@chrisguidry chrisguidry deleted the sync-depends-support branch October 24, 2025 19:08
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.

4 participants