Skip to content

Fix bug in deployer reload logic #2533

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

Merged
merged 1 commit into from
Aug 11, 2025
Merged

Conversation

talsperre
Copy link
Collaborator

Make to_reload a property that can be overwritten by other implementations like maestro etc. This is because, sometimes we need to reload extension specific modules.

@talsperre talsperre requested review from mt-ob and Copilot August 3, 2025 05:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request fixes a bug in the deployer reload logic by converting hardcoded module reload behavior into a configurable property. The change allows extension-specific implementations (like maestro) to override which modules get reloaded during deployer initialization.

Key changes:

  • Convert hardcoded to_reload list to a property method that can be overridden
  • Move module list definition from __init__ method to a dedicated property with documentation

Comment on lines +104 to +105
This is used to ensure that the CLI is in a clean state before
deploying the flow.
Copy link
Preview

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

The docstring should clarify that this property is intended to be overridden by subclasses for extension-specific module reloading, as mentioned in the PR description.

Suggested change
This is used to ensure that the CLI is in a clean state before
deploying the flow.
This property is intended to be overridden by subclasses for extension-specific
module reloading. Subclasses should override this to specify which modules
need to be reloaded to ensure the CLI is in a clean state before deploying the flow.

Copilot uses AI. Check for mistakes.

@savingoyal savingoyal merged commit c588777 into master Aug 11, 2025
28 of 29 checks passed
@savingoyal savingoyal deleted the dev/deployer/fix-param-bug branch August 11, 2025 19:31
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.

3 participants