Skip to content

Implement #162: MADSciClientMixin - #183

Merged
RyanTheRobothead merged 16 commits into
mainfrom
claude/implement-issue-162-015zDLPaqvtrf6XaaHXCnLNT
Dec 5, 2025
Merged

Implement #162: MADSciClientMixin#183
RyanTheRobothead merged 16 commits into
mainfrom
claude/implement-issue-162-015zDLPaqvtrf6XaaHXCnLNT

Conversation

@RyanTheRobothead

@RyanTheRobothead RyanTheRobothead commented Nov 18, 2025

Copy link
Copy Markdown
Member

PR Info

Developer Checklists

I have:

  • Run Pre-commit and Unit Tests, and ensured that they pass
  • Created or updated documentation relevant to your change
  • Created or updated unit tests relevant to your change

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
@RyanTheRobothead RyanTheRobothead changed the title Implement solution for GitHub issue 162: MADSciClientMixin Implement #162: MADSciClientMixin Nov 18, 2025
@github-actions

github-actions Bot commented Nov 20, 2025

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/madsci_client/madsci/client
  __init__.py
  client_mixin.py 235, 242, 283, 320, 327, 363, 377, 392-430, 464, 482, 500, 515-521
  resource_client.py 654-657
  src/madsci_common/madsci/common
  manager_base.py
  src/madsci_experiment_application/madsci/experiment_application
  experiment_application.py 86, 131-133
  src/madsci_node_module/madsci/node_module
  abstract_node_module.py 80, 380
  src/madsci_workcell_manager/madsci/workcell_manager
  workcell_engine.py 610-616
  workcell_server.py 87, 158
Project Total  

This report was generated by python-coverage-comment-action

@RyanTheRobothead RyanTheRobothead self-assigned this Dec 4, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 MadsciClientMixin with lazy client initialization and configuration management
  • Integrates the mixin into AbstractNode, AbstractManagerBase, WorkcellManager, and ExperimentApplication
  • 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.

Comment thread example_lab/notebooks/node_notebook.ipynb Outdated
Comment thread example_lab/notebooks/experiment_notebook.ipynb Outdated
@RyanTheRobothead
RyanTheRobothead merged commit c8a8356 into main Dec 5, 2025
3 checks passed
@RyanTheRobothead
RyanTheRobothead deleted the claude/implement-issue-162-015zDLPaqvtrf6XaaHXCnLNT branch December 27, 2025 15:03
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.

Feature: Add MadsciClientMixin helper class

3 participants