feat(workflow): add support for different workflows; workflow registry by yashlamba · Pull Request #41 · inveniosoftware/orcha

yashlamba · 2026-04-21T11:49:55Z

This PR is to refactor a few things:

Support multiple workflows, which we register on application start.
Param validation for each workflow. We use a workflow "builder" for the request.

Following the PR, I'll create a separate PR to integrate ty, since it's time to be a bit stricter with types.

mairasalazar

just a couple of small comments

…model

slint

Happy with the registry plumbing, but I have some gripes still with naming and ergonomics, which I think we should get right/discuss.

slint · 2026-04-28T22:20:37Z

+    workflow_fn: Any
+    """Entry-point method of the ``@workflow.defn`` class."""
+
+    params_model: type[BaseModel]
+    """Pydantic model used to validate workflow-specific input params."""
+
+    request_builder: Callable[[WorkflowContext, BaseModel], BaseModel]


minor: I left a comment in a past PR about a possible way to structure workflows in a way that encapsulates some of its parts in the existing workflow class that Temporal sort of "enforces" us to have.

I think this WorkflowSpec class is a good "breakdown" of this, but it feels a lot like we're passing a lot of things around that could just be attached in the Workflow class itself. Some things I'm skeptical about:

For workflow_fn we pass e.g. ExtractMetadata.run: why not just pass ExtractMetadata, which already has the Temporal decorators (@workflow.defn and @workflow.run) applied to the function? I know that Temporal docs recommend either passing the name of the workflow as a string (e.g. "ExtractMetadata") or the "run" func (e.g. ExtractMetadata.run), but this feels redundant...

For params_model, see my original comment in the PR. I feel like it becomes a very non-ergonomic way of "typing" a workflow's inputs, which are kind of already defined/implied in the signature.

For request_builder, from what I understand, this is to "bundle" in the input params our workflow ID + tenant ID. If we are to be defining this kind of function for each workflow (and having it basically always look the same), it should just be a (class)method or similar in the Workflow class.

The above for me boil down to that composing these elements into the spec doesn't add value, since we won't ever want to swap them as part of testing or redefinitions of workflows.

We could make better use of the workflow class, and define a mixin or base class which allows us to enforce some of these fields.

slint · 2026-04-28T22:25:15Z

+        workflow_type=body.workflow_type,
        status=WorkflowStatus.PROCESSING,
-        url=body.url,
+        params=params.model_dump(mode="json"),


minor/question: do you need to serialize here the Pydantic model to JSON? Maybe we need to configure Pydantic support on the Temporal client?

slint · 2026-04-28T22:28:45Z

        raise HTTPException(status_code=500, detail="Could not create workflow")

    try:
+        workflow_request = spec.request_builder(


nit: in this FastAPI request context, naming another thing a "request" feels a bit confusing. Based on the temporal types, this is either a workflow "handle" or maybe just "params" or workflow_args

slint · 2026-04-28T22:30:57Z

+    pages: list[int] | None = Field(default_factory=lambda: [1, 2])


 class ExtractMetadataWorkflowRequest(BaseModel):


minor: shouldn't this class be a subclass of WorkflowContext?

OliverGeneser · 2026-05-12T09:13:40Z

+class WorkflowSpec:
+    """Describes a Temporal workflow that the API can dispatch."""
+
+    workflow_fn: Any


I guess Temporal doesn't expose the type for workflows? Maybe we could make it a bit stricter with Callable and that it need to return Awaitable?

Let's make it workflow_cls instead, since we register those with Temporal. I'll move and see if that can be typed.

OliverGeneser · 2026-05-12T09:18:34Z

+
+# Import workflow modules so their register_workflow() calls execute.
+# To add a new workflow type, add an import here.
+import app.workflows.extract_metadata_workflow  # noqa: F401


Maybe we could add a register_all_workflows function in registry.py to make workflow registration explicit instead of relying on import side effects?

We discussed this IRL, Alex and I, and I think I'll do that now.

Also, we don't have support for task queues; we can only ship one kind of worker. I'll try to fix it and add a default q for now.

OliverGeneser · 2026-05-12T09:21:56Z

@@ -122,9 +129,7 @@ async def create(
            session.commit()
        except SQLAlchemyError:
            pass
-        raise HTTPException(
-            status_code=500, detail="Could not start extraction workflow"
-        )
+        raise HTTPException(status_code=500, detail="Could not start workflow")


We should add logger so we have proper production logs

Coming soon!

yashlamba marked this pull request as draft April 21, 2026 12:21

yashlamba force-pushed the worflow-registry branch 4 times, most recently from fac0afb to ba4cc90 Compare April 27, 2026 15:01

feat(workflow): add support for different workflows; workflow registry

fc88d76

mairasalazar reviewed Apr 28, 2026

View reviewed changes

Comment thread app/workflows/registry.py Outdated

Comment thread app/routers/workflows.py Outdated

yashlamba added 2 commits April 28, 2026 14:00

refactor: move result schemas separately

d21f6c7

refactor: use builder pattern for workflow requests; remove url from …

b49b303

…model

yashlamba force-pushed the worflow-registry branch from ba4cc90 to b49b303 Compare April 28, 2026 13:28

yashlamba marked this pull request as ready for review April 28, 2026 13:29

yashlamba added this to Sprint Q2 2026 ☀️ Apr 28, 2026

yashlamba moved this to In review 🔍 in Sprint Q2 2026 ☀️ Apr 28, 2026

yashlamba requested a review from mairasalazar April 28, 2026 13:35

yashlamba assigned ptamarit Apr 28, 2026

yashlamba requested a review from slint April 28, 2026 13:35

yashlamba unassigned ptamarit Apr 28, 2026

yashlamba requested a review from ptamarit April 28, 2026 13:35

ptamarit approved these changes Apr 28, 2026

View reviewed changes

slint reviewed Apr 29, 2026

View reviewed changes

refactor: remove builder; separate param models

dd475c6

yashlamba requested a review from OliverGeneser May 12, 2026 08:20

OliverGeneser approved these changes May 12, 2026

View reviewed changes

		pages: list[int] \| None = Field(default_factory=lambda: [1, 2])


		class ExtractMetadataWorkflowRequest(BaseModel):

Conversation

yashlamba commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mairasalazar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

slint left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yashlamba commented Apr 21, 2026 •

edited

Loading