Implement #162: MADSciClientMixin - #183
Merged
RyanTheRobothead merged 16 commits intoDec 5, 2025
Merged
Conversation
Implements issue #162 by creating a reusable mixin class that manages MADSci client initialization and lifecycle across all components. ## New Features - **MadsciClientMixin**: Central mixin class for managing all 7 MADSci clients - EventClient, ResourceClient, DataClient, ExperimentClient - WorkcellClient, LocationClient, LabClient - Lazy initialization with property-based access - Flexible configuration via context, explicit URLs, or direct injection - EventClient sharing across dependent clients - Comprehensive unit test suite ## Refactored Components 1. **AbstractNode**: Now uses mixin for client management - Simplified _configure_clients() method - Maintains backward compatibility with logger alias - Declares REQUIRED_CLIENTS: ["event", "resource", "data"] 2. **AbstractManagerBase**: Integrated with mixin - Updated setup_logging() to use mixin's event_client - Inherits all client management functionality - Maintains existing API surface 3. **ExperimentApplication**: Uses inherited mixin from AbstractNode - Removed manual client initialization - Declares OPTIONAL_CLIENTS for additional manager clients - Cleaner __init__ method with better documentation 4. **WorkcellManager**: Leverages mixin via AbstractManagerBase - Removed manual DataClient/LocationClient creation - Uses setup_clients() in initialize() - Declares REQUIRED_CLIENTS: ["event", "data", "location"] ## Implementation Details - All client properties use lazy initialization - EventClient is always initialized first (other clients depend on it) - Support for client-specific configuration (retry settings, working directory, etc.) - Proper type hints with ClassVar for client lists - Reduced code complexity via helper methods - All code passes ruff linting and formatting ## Benefits - **DRY**: Eliminates duplicate client initialization code across components - **Consistency**: Standardized client setup patterns - **Maintainability**: Single source of truth for client management - **Testability**: Easy mock injection for unit tests - **Flexibility**: Supports multiple configuration strategies - **Backward Compatible**: No breaking changes to existing APIs ## Testing - Comprehensive unit test suite for MadsciClientMixin - Tests cover all client types, initialization patterns, and configuration options - All modified code passes ruff checks Closes #162
- Rewrote tests to be simpler and more focused on actual functionality - Fixed _init_client_batch to avoid eager property access - Tests now use direct mock injection instead of complex nested patches - All 22 mixin tests pass successfully - Removed dependency on context mocking in mixin itself
MADSciClientMixinMADSciClientMixin
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements the MadsciClientMixin class to centralize client lifecycle management across the MADSci ecosystem, addressing issue #162. The mixin provides lazy initialization, context-based auto-configuration, and explicit URL overrides for all MADSci service clients (event, resource, data, experiment, workcell, location, and lab).
Key Changes:
- Introduces
MadsciClientMixinwith lazy client initialization and configuration management - Integrates the mixin into
AbstractNode,AbstractManagerBase,WorkcellManager, andExperimentApplication - Updates test infrastructure to patch clients at the mixin level rather than individual modules
- Renames testing targets from "integration_tests" to "e2e_tests" for clarity
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/madsci_client/madsci/client/client_mixin.py | New mixin class providing centralized client lifecycle management with lazy initialization and flexible configuration |
| src/madsci_node_module/madsci/node_module/abstract_node_module.py | Inherits from MadsciClientMixin and updates _configure_clients to use the mixin's setup_clients() method |
| src/madsci_common/madsci/common/manager_base.py | Adds MadsciClientMixin to AbstractManagerBase and updates setup_logging to use lazy client initialization |
| src/madsci_workcell_manager/madsci/workcell_manager/workcell_server.py | Declares REQUIRED_CLIENTS and uses setup_clients() instead of direct client instantiation |
| src/madsci_experiment_application/madsci/experiment_application/experiment_application.py | Declares OPTIONAL_CLIENTS and removes manual client initialization in favor of mixin's lazy properties |
| src/madsci_client/tests/test_client_mixin.py | Comprehensive test suite for the new mixin covering initialization, setters, setup, configuration, and teardown |
| src/madsci_client/tests/test_workcell_client.py | Updates mocks to inject event_client and patch at mixin level instead of module level |
| src/madsci_experiment_application/tests/test_experiment_application.py | Updates patch paths to target madsci.client.client_mixin instead of experiment_application module |
| src/madsci_client/madsci/client/resource_client.py | Bug fix: handles string resource IDs in local mode by converting to resource objects |
| src/madsci_client/madsci/client/init.py | Exports MadsciClientMixin and LabClient for public API |
| example_lab/notebooks/node_notebook.ipynb | Documents mixin usage and client access patterns; updates examples to use event_client |
| example_lab/notebooks/experiment_notebook.ipynb | Documents automatic client management benefits and updates examples |
| compose.yaml | Adds example_lab/.env to env_file list for environment variable loading |
| .justfile | Renames integration test targets to e2e_tests for clarity |
| .github/workflows/e2e_tests.yml | Updates workflow name and target to use e2e_tests |
| README.md | Updates Docker prerequisite description to mention e2e tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…e-162-015zDLPaqvtrf6XaaHXCnLNT
RyanTheRobothead
deleted the
claude/implement-issue-162-015zDLPaqvtrf6XaaHXCnLNT
branch
December 27, 2025 15:03
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Info
MadsciClientMixinhelper class #162Developer Checklists
I have: