refactor: orchestrate #220
Open
+453
−388
Conversation
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
The refactoring moves general/explore orchestration function to private modules for clarity. However we don't want to expose this private modules external - want to keep the access point orchestrate.py
orchestrate_explore_operation
…operation # Conflicts: # .secrets.baseline # orchestrator/modules/operators/orchestrate.py
…operation # Conflicts: # orchestrator/modules/operators/orchestrate.py
Remove unused parameters.
Not needed as its we must use the one given by discover_space.project_context
from orchestrate and orchestrate_exploer_operation. it was never not None so not required to be a parameter
execid -> namespace
Originally required as it held common function for two sub-classes. However, one of those was removed meaning we don't need a base class anymore and can simplify the code
From orchestrate to general_orchestration. This is because - orchestrate_explore_operation already does this so it doesn't need it from orchestrate - the information is not passed from orchestrate to orchestrate_general_operation - orchestrate_general_operation should have this info but it needs to acquire it itself in case the function is called directly (not via orchestrate) Hence, it's redundant to have this code here. The correct place its needed is in orchestrate_general_operation
Member
|
Checks Summary Last run: 2025-11-21T17:17:01.590Z Code Risk Analyzer vulnerability scan found 2 vulnerabilities:
|
Extract functions for getting and validating actuator configurations. Rename methods. The reason is that as-is simplified get/validate is tied to DiscoveryOperationResourceConfiguration but this constrains where it has to happen to the existence of an instance of the class.
now takes more basic components and is called create_operation_and_add_to_metastore
Update parameters to take operator module/parameters/info rather than having them bundled in DiscoveryOperationConfiguration instance Update to new create_operation_and_add_to_metastore interface.
Update parameters to take operator module/parameters/info rather than a DiscoveryOperationResourceConfiguration instance. This is so the parameters are as similar as possible to orchestrate_general_operation make the paths to each more unififed. Remove namespace parameter = the orchestrate function will generate its own namespace for its operation Simplify return to OperationOutput - other instance returned are deducible from this or were input Remove orchestrate_operation_function_wrapper - no longer necessary due to change in orchestrate_explore_operation input/output
Change parameters to take operator_module and parameters instead of requiring them to be bundled in DiscoveryOperationResourceConfiguration
To allow communicating it to a general operation that wants to create ray actors/remotes
No need to call wrapper or set namespace explicitly.
Also update to new run_operation_harness
- Don't namespace at orchestrate level, let the orchestration functions set namespace - Update to new orchestrate_*_operation interfaces - Remove orchestrate_operation_function - no longer necessary
Signed-off-by: Michael Johnston <[email protected]>
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.
This PR:
orchestrate.orchestrate_operation_functionand_orchestrate_explore_operation.orchestrate_explore_operation_function_wrapperTowards fixing #200 where we need to be able to isolate each operation. It will be easier if the
orchestrate_*_operationfunctions contain same information, control same orchestration elements, and there are no extraneous functions to have to deal with.