TFX pipeline test (Testing Frameworks) #1986
Conversation
tfx/orchestration/pipeline.py
Outdated
| def set_executor(self, component_id: Text, executor_factory: Type[FakeComponentExecutorFactory]) -> None: | ||
| for component in self._components: | ||
| if component_id is component.id: | ||
| self.mock_executor_spec[component_id] = executor_factory() |
There was a problem hiding this comment.
How about assigning executor_spec directly here? For example,
component.executor_spec = some_executor_spec
Then we might not need to add a new argument for mock executors in other APIs.
There was a problem hiding this comment.
+1.
This will work.
component.executor_spec = ExecutorClassSpec(executor_class)
tfx/orchestration/pipeline.py
Outdated
| @@ -189,3 +200,23 @@ def components(self, components: List[base_node.BaseNode]): | |||
| # has all its dependencies visited. | |||
| if len(self._components) < len(deduped_components): | |||
| raise RuntimeError('There is a cycle in the pipeline') | |||
|
|
|||
| def set_executor(self, component_id: Text, executor_factory: Type[FakeComponentExecutorFactory]) -> None: | |||
There was a problem hiding this comment.
I think that it would be generally better to pass a Factory instance as an argument, rather than a type. If executor_factory is either FakeComponentExecutorFactory instance or a real executor class type, it would be very helpful. For example, we might be able to use a custom made executor class for one component and use mocks for other components.
It means that we might want the type of executor_factory is FakeComponentExecutorFactory OR Type[BaseExecutor].
But IIUC, what we really use is executor_spec (For example, FakeExecutorClassSpec), So I'd like to suggest change argument to ExecutorSpec, then we might be able to assign it directly to the component.
This reverts commit 15b4fd9.
PiperOrigin-RevId: 319939150
PiperOrigin-RevId: 321081977
PiperOrigin-RevId: 319701322
PiperOrigin-RevId: 319701322
No description provided.