-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: orchestrate #220
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
base: main
Are you sure you want to change the base?
Changes from all commits
e525b90
08f55ff
8842423
de000a0
9e9c060
7951172
40f0542
4cba376
0e53c76
c97df6d
a0a0bc6
dced8ce
bb7a007
f17530d
f14973a
f95dbac
4576ca2
1bb9e67
177824f
c7943e1
3ed858c
35e0721
547d3a1
5c0d194
1814f2e
15136f3
2fc4a24
cb00a17
1e35ee2
b561455
b12afe8
72ea4bd
ffb3b4f
08f13da
41c6265
1019893
d2f8ea9
4bffb1c
f25e73c
656505c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,141 @@ class DiscoveryOperationEnum(enum.Enum): | |
| EXPORT = "export" | ||
|
|
||
|
|
||
| def get_actuator_configurations( | ||
| project_context: ProjectContext, actuator_configuration_identifiers: list[str] | ||
| ) -> list[ActuatorConfiguration]: | ||
| """Retrieves actuator configurations from the metastore | ||
|
|
||
| Fetches ActuatorConfiguration resources from the metastore using the provided | ||
| identifiers and validates that each actuator has at most one configuration. | ||
|
|
||
| Params: | ||
| project_context: Project context for connecting to the metastore | ||
| actuator_configuration_identifiers: List of identifiers for actuator | ||
| configuration resources to retrieve | ||
|
|
||
| Returns: | ||
| List of ActuatorConfiguration instances retrieved from the metastore | ||
|
|
||
| Raises: | ||
| ValueError: If more than one ActuatorConfiguration references the same actuator | ||
| """ | ||
| import orchestrator.metastore.sqlstore | ||
|
|
||
| sql = orchestrator.metastore.sqlstore.SQLStore(project_context=project_context) | ||
|
|
||
| actuator_configurations = [ | ||
| sql.getResource( | ||
| identifier=identifier, | ||
| kind=CoreResourceKinds.ACTUATORCONFIGURATION, | ||
| raise_error_if_no_resource=True, | ||
| ).config | ||
| for identifier in actuator_configuration_identifiers | ||
| ] | ||
|
|
||
| actuator_identifiers = {conf.actuatorIdentifier for conf in actuator_configurations} | ||
| if len(actuator_identifiers) != len(actuator_configuration_identifiers): | ||
| raise ValueError("Only one ActuatorConfiguration is permitted per Actuator") | ||
|
|
||
| return actuator_configurations | ||
|
|
||
|
|
||
| def validate_actuator_configurations_against_space_configuration( | ||
| actuator_configurations: list[ActuatorConfiguration], | ||
| discovery_space_configuration: DiscoverySpaceConfiguration, | ||
| ) -> list[ActuatorConfiguration]: | ||
| """Validates that actuator configurations are compatible with a discovery space | ||
|
|
||
| Checks that all actuators referenced in the actuator configurations are used | ||
| in the experiments defined in the discovery space configuration. | ||
|
|
||
| Params: | ||
| actuator_configurations: List of actuator configurations to validate | ||
| discovery_space_configuration: The discovery space configuration to validate against | ||
|
|
||
| Returns: | ||
| The same list of actuator_configurations if validation passes | ||
|
Comment on lines
+93
to
+94
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth to return the same exact input parameter? We could just have this return nothing and just raise an error if validation fails |
||
|
|
||
| Raises: | ||
| ValueError: If any actuator identifier in actuator_configurations does not | ||
| appear in the experiments of the discovery space | ||
| """ | ||
| actuator_identifiers = {conf.actuatorIdentifier for conf in actuator_configurations} | ||
|
|
||
| # Check the actuators configurations refer to actuators used in the MeasurementSpace | ||
| # The experiment identifiers are in two different locations | ||
| if isinstance( | ||
| discovery_space_configuration.experiments, MeasurementSpaceConfiguration | ||
| ): | ||
| experiment_actuator_identifiers = { | ||
| experiment.actuatorIdentifier | ||
| for experiment in discovery_space_configuration.experiments.experiments | ||
| } | ||
| else: | ||
| experiment_actuator_identifiers = { | ||
| experiment.actuatorIdentifier | ||
| for experiment in discovery_space_configuration.experiments | ||
| } | ||
|
|
||
| if not experiment_actuator_identifiers.issuperset(actuator_identifiers): | ||
| raise ValueError( | ||
| f"Actuator Identifiers {actuator_identifiers} must appear in the experiments of its space" | ||
| ) | ||
|
|
||
| return actuator_configurations | ||
|
|
||
|
|
||
| def validate_actuator_configuration_ids_against_space_ids( | ||
| actuator_configuration_identifiers: list[str], | ||
| space_identifiers: list[str], | ||
| project_context: ProjectContext, | ||
| ): | ||
| """Validates actuator configuration identifiers against space identifiers | ||
|
|
||
| Retrieves actuator configurations and space configurations from the metastore, | ||
| then validates that all actuator configurations are compatible with all specified | ||
| discovery spaces. | ||
|
|
||
| Params: | ||
| actuator_configuration_identifiers: List of actuator configuration resource | ||
| identifiers to validate | ||
| space_identifiers: List of discovery space resource identifiers to validate against | ||
| project_context: Project context for connecting to the metastore | ||
|
|
||
| Returns: | ||
| List of ActuatorConfiguration instances that were validated | ||
|
|
||
| Raises: | ||
| ValueError: If any actuator configuration is not compatible with any of the | ||
| discovery spaces, or if more than one ActuatorConfiguration references | ||
| the same actuator | ||
| """ | ||
| import orchestrator.metastore.sqlstore | ||
|
|
||
| sql = orchestrator.metastore.sqlstore.SQLStore(project_context=project_context) | ||
| space_configurations: list[DiscoverySpaceConfiguration] = [ | ||
| sql.getResource( | ||
| identifier=identifier, | ||
| kind=CoreResourceKinds.DISCOVERYSPACE, | ||
| raise_error_if_no_resource=True, | ||
| ).config | ||
|
Comment on lines
+154
to
+158
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can also raise |
||
| for identifier in space_identifiers | ||
| ] | ||
|
|
||
| actuator_configurations = get_actuator_configurations( | ||
| project_context=project_context, | ||
| actuator_configuration_identifiers=actuator_configuration_identifiers, | ||
| ) | ||
|
|
||
| for config in space_configurations: | ||
| validate_actuator_configurations_against_space_configuration( | ||
| actuator_configurations=actuator_configurations, | ||
| discovery_space_configuration=config, | ||
| ) | ||
|
|
||
| return actuator_configurations | ||
|
|
||
|
|
||
| class OperatorModuleConf(ModuleConf): | ||
| moduleType: ModuleTypeEnum = pydantic.Field(default=ModuleTypeEnum.OPERATION) | ||
|
|
||
|
|
@@ -128,10 +263,8 @@ class DiscoveryOperationConfiguration(pydantic.BaseModel): | |
| ) | ||
|
|
||
|
|
||
| class BaseOperationRunConfiguration(pydantic.BaseModel): | ||
| """Field shared by OrchestratorRunConfiguration and OperationResourceConfiguration | ||
|
|
||
| both are models used to run an operation""" | ||
| class DiscoveryOperationResourceConfiguration(pydantic.BaseModel): | ||
| """Pydantic model used to define an operation""" | ||
|
|
||
| operation: DiscoveryOperationConfiguration | ||
| metadata: ConfigurationMetadata = pydantic.Field( | ||
|
|
@@ -140,13 +273,27 @@ class BaseOperationRunConfiguration(pydantic.BaseModel): | |
| "Two optional keys that are used by convention are name and description", | ||
| ) | ||
| actuatorConfigurationIdentifiers: list[str] = pydantic.Field(default=[]) | ||
| spaces: list[str] = pydantic.Field( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be consistent with |
||
| description="List of ids of the spaces the operation will be applied to" | ||
| ) | ||
| model_config = ConfigDict( | ||
| extra="forbid", | ||
| json_schema_extra={ | ||
| "version": importlib.metadata.version(distribution_name="ado-core") | ||
| }, | ||
| ) | ||
|
|
||
| @pydantic.field_validator("spaces") | ||
| def check_space_set(cls, value): | ||
| """Checks at least one space identifier has been given""" | ||
|
|
||
| if len(value) == 0: | ||
| raise ValueError( | ||
| "You must provide at least one space identifier to an operation" | ||
| ) | ||
|
|
||
| return value | ||
|
|
||
|
Comment on lines
+286
to
+296
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be replaced by adding |
||
| def get_actuatorconfigurations( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is kind of useless at the moment - if we push early exit condition for empty |
||
| self, project_context: ProjectContext | ||
| ) -> list[ActuatorConfiguration]: | ||
|
|
@@ -163,115 +310,49 @@ def get_actuatorconfigurations( | |
| Raises: ValueError if there is more than one ActuatorConfigurationResource references the same actuator | ||
| """ | ||
|
|
||
| import orchestrator.metastore.sqlstore | ||
|
|
||
| if not self.actuatorConfigurationIdentifiers: | ||
| return [] | ||
|
|
||
| sql = orchestrator.metastore.sqlstore.SQLStore(project_context=project_context) | ||
|
|
||
| actuator_configurations = [ | ||
| sql.getResource( | ||
| identifier=identifier, | ||
| kind=CoreResourceKinds.ACTUATORCONFIGURATION, | ||
| raise_error_if_no_resource=True, | ||
| ).config | ||
| for identifier in self.actuatorConfigurationIdentifiers | ||
| ] | ||
|
|
||
| actuator_identifiers = { | ||
| conf.actuatorIdentifier for conf in actuator_configurations | ||
| } | ||
| if len(actuator_identifiers) != len(self.actuatorConfigurationIdentifiers): | ||
| raise ValueError("Only one ActuatorConfiguration is permitted per Actuator") | ||
|
|
||
| return actuator_configurations | ||
|
|
||
| def validate_actuatorconfigurations_against_space( | ||
| self, | ||
| project_context: ProjectContext, | ||
| discoverySpaceConfiguration: DiscoverySpaceConfiguration, | ||
| ) -> list[ActuatorConfiguration]: | ||
|
|
||
| actuator_configurations = self.get_actuatorconfigurations( | ||
| project_context=project_context | ||
| return get_actuator_configurations( | ||
| project_context=project_context, | ||
| actuator_configuration_identifiers=self.actuatorConfigurationIdentifiers, | ||
| ) | ||
| actuator_identifiers = { | ||
| conf.actuatorIdentifier for conf in actuator_configurations | ||
| } | ||
|
|
||
| # Check the actuators configurations refer to actuators used in the MeasurementSpace | ||
| # The experiment identifiers are in two different locations | ||
| if isinstance( | ||
| discoverySpaceConfiguration.experiments, MeasurementSpaceConfiguration | ||
| ): | ||
| experiment_actuator_identifiers = { | ||
| experiment.actuatorIdentifier | ||
| for experiment in discoverySpaceConfiguration.experiments.experiments | ||
| } | ||
| else: | ||
| experiment_actuator_identifiers = { | ||
| experiment.actuatorIdentifier | ||
| for experiment in discoverySpaceConfiguration.experiments | ||
| } | ||
|
|
||
| if not experiment_actuator_identifiers.issuperset(actuator_identifiers): | ||
| raise ValueError( | ||
| f"Actuator Identifiers {actuator_identifiers} must appear in the experiments of its space" | ||
| ) | ||
|
|
||
| return actuator_configurations | ||
|
|
||
|
|
||
| class DiscoveryOperationResourceConfiguration(BaseOperationRunConfiguration): | ||
|
|
||
| spaces: list[str] = pydantic.Field( | ||
| description="The spaces the operation will be applied to" | ||
| ) | ||
|
|
||
| @pydantic.field_validator("spaces") | ||
| def check_space_set(cls, value): | ||
| """Checks at least one space identifier has been given""" | ||
|
|
||
| if len(value) == 0: | ||
| raise ValueError( | ||
| "You must provide at least one space identifier to an operation" | ||
| ) | ||
|
|
||
| return value | ||
|
|
||
| def validate_actuatorconfigurations( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also seems redundant now |
||
| self, project_context: ProjectContext | ||
| ) -> list[ActuatorConfiguration]: | ||
| """Gets and valdidates the actuator configuration resources referenced by actuatorConfigurationIdentifiers from the metastore if any | ||
|
|
||
| from orchestrator.core.discoveryspace.space import DiscoverySpace | ||
| This also requires getting the configuration of the discovery space | ||
|
|
||
| actuator_configurations: list[ActuatorConfiguration] = [] | ||
| for space in self.spaces: | ||
| discovery_space = DiscoverySpace.from_stored_configuration( | ||
| project_context=project_context, | ||
| space_identifier=space, | ||
| ) | ||
| Params: | ||
| project_context: Information for connection to the metastore | ||
|
|
||
| actuator_configurations.extend( | ||
| super().validate_actuatorconfigurations_against_space( | ||
| project_context=project_context, | ||
| discoverySpaceConfiguration=discovery_space.config, | ||
| ) | ||
| ) | ||
| Returns: | ||
| A list of ActuatorConfigurationResource instance. The list will be empty if | ||
| there are no actuatorConfigurationIdentifiers. | ||
|
|
||
| return actuator_configurations | ||
|
|
||
| Raises: ValueError if more than one ActuatorConfigurationResource references the same actuator | ||
| """ | ||
|
|
||
| return validate_actuator_configuration_ids_against_space_ids( | ||
| actuator_configuration_identifiers=self.actuatorConfigurationIdentifiers, | ||
| space_identifiers=self.spaces, | ||
| project_context=project_context, | ||
| ) | ||
|
|
||
| class FunctionOperationInfo(pydantic.BaseModel): | ||
| """Class for holding information for operations executed via operator functions | ||
|
|
||
| Operators implemented as functions may need additional information. | ||
| Rather that have these as multiple params we gather them in this model""" | ||
| class FunctionOperationInfo(pydantic.BaseModel): | ||
| """Class for providing information to operator functions""" | ||
|
|
||
| metadata: ConfigurationMetadata = pydantic.Field( | ||
| default=ConfigurationMetadata(), | ||
| description="User defined metadata about the configuration. A set of keys and values. " | ||
| "Two optional keys that are used by convention are name and description", | ||
| ) | ||
| actuatorConfigurationIdentifiers: list[str] = pydantic.Field(default=[]) | ||
| namespace: str | None = pydantic.Field( | ||
| description="The namespace the operation should create ray workers/actors in", | ||
| default=None, | ||
| ) | ||
|
Comment on lines
+355
to
+358
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd call this more explicitly |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code can also raise a
ResourceDoesNotExistError- I think I forgot to update the docstring for the interface method