WF-IMPL-080: providers.py wiring + Configuration knobs#503
Merged
Conversation
Refs #491, #495. * Add WF_ARM_ENDPOINT / WF_CONNECTOR_ENDPOINT / WF_OUTBOUND_RPC_TIMEOUT_MS constants to providers.py with the design-locked names from design/components/workflow-service/design.md. * Add _publisher_env_active helper, _resolve_outbound_rpc_timeout_seconds (positive-int ms → seconds, eager ValueError at startup), and _build_activity_client / _build_connector_client factories that swap between NoopActivityRuntimeClient / NoopConnectorClient (dev / test sidecar-free) and the WF-IMPL-077 / WF-IMPL-078 DaprActivityRuntimeClient / DaprConnectorClient when the corresponding env var is set. * Rewrite load_run_components so a single lifespan-owned httpx.AsyncClient is built only when at least one Dapr consumer is active (publisher, ARM adapter, or Connector adapter) and the same client instance is threaded into every adapter — no second client, no second socket pool. Caller-supplied overrides still short-circuit the env-var-driven build, preserving the existing Fake-injection contract. * Construct the default WorkflowRuntime with the resolved ARM / Connector clients so the WF-IMPL-079 outbound-activity bridges register against the production adapters at runtime.start(). * Extend tests/test_providers.py with the four-branch env matrix (both unset, ARM only, Connector only, both set), pin the single-client identity invariant against RunComponents.dapr_http_client, assert the Noop fallback when env vars are unset, and exercise _resolve_outbound_rpc_timeout_seconds happy / sad paths (default, positive int, non-integer, non-positive, end-to-end timeout threaded into the Dapr adapter). * Document WF_ARM_ENDPOINT, WF_CONNECTOR_ENDPOINT, and WF_OUTBOUND_RPC_TIMEOUT_MS in docs/developers/workflow-api.md Configuration table (call out the single socket pool, the Noop fallback, and the startup-fail-fast on bad values).
toddysm
added
type:implementation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR wires the WF-IMPL-077/078 outbound Dapr clients into workflow-service’s load_run_components() behind new env knobs, sharing a single lifespan-owned httpx.AsyncClient across all outbound Dapr consumers, and documents/tests the new configuration surface.
Changes:
- Add
WF_ARM_ENDPOINT,WF_CONNECTOR_ENDPOINT, andWF_OUTBOUND_RPC_TIMEOUT_MSconfiguration (with fail-fast parsing) and updateload_run_components()to build/share onehttpx.AsyncClientwhen any outbound Dapr path is active. - Update lifecycle publisher wiring to reuse the shared
httpx.AsyncClientinstead of owning its own. - Add unit tests covering the env-var combinations, override short-circuiting, and timeout parsing; update developer docs configuration table.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
src/services/workflow-service/src/custos_workflow/providers.py |
Adds new env knobs/timeout parsing and rewires component construction to share a single lifespan-owned httpx.AsyncClient across publisher + outbound Dapr clients. |
src/services/workflow-service/tests/test_providers.py |
Adds tests for endpoint env-var branches, shared HTTP client identity, overrides, and timeout parsing/wiring. |
docs/developers/workflow-api.md |
Extends configuration table with the new outbound Dapr endpoint + timeout settings. |
Comments suppressed due to low confidence (3)
docs/developers/workflow-api.md:496
- The added config rows include outbound-RPC and Dapr endpoints, which are owned by the workflow runtime/providers layer rather than the API Adapter specifically. Calling these 'API Adapter sub-module' env vars is misleading in this section; it should refer to the workflow-service (or workflow-service app) configuration instead.
The API Adapter sub-module reads six environment variables
directly; the rest of the workflow-service host's config
([`README.md` § Configuration](../../src/services/workflow-service/README.md#configuration))
remains the source of truth for the variables the validator
collaborators (Catalog client, Dapr Workflow component, etc.)
bind against.
docs/developers/workflow-api.md:496
- The added config rows include outbound-RPC and Dapr endpoints, which are owned by the workflow runtime/providers layer rather than the API Adapter specifically. Calling these 'API Adapter sub-module' env vars is misleading in this section; it should refer to the workflow-service (or workflow-service app) configuration instead.
The API Adapter sub-module reads six environment variables
directly; the rest of the workflow-service host's config
([`README.md` § Configuration](../../src/services/workflow-service/README.md#configuration))
remains the source of truth for the variables the validator
collaborators (Catalog client, Dapr Workflow component, etc.)
bind against.
docs/developers/workflow-api.md:496
- The added config rows include outbound-RPC and Dapr endpoints, which are owned by the workflow runtime/providers layer rather than the API Adapter specifically. Calling these 'API Adapter sub-module' env vars is misleading in this section; it should refer to the workflow-service (or workflow-service app) configuration instead.
The API Adapter sub-module reads six environment variables
directly; the rest of the workflow-service host's config
([`README.md` § Configuration](../../src/services/workflow-service/README.md#configuration))
remains the source of truth for the variables the validator
collaborators (Catalog client, Dapr Workflow component, etc.)
bind against.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses Copilot review feedback on #503: * Add outbound_activity_client and outbound_connector_client fields to RunComponents. These hold the WF-IMPL-077 / WF-IMPL-078 Dapr async clients (or their Noop fallbacks) and are threaded into WorkflowRuntime(activity_runtime_client=..., connector_client=...) so the WF-IMPL-079 bridge activities register against them. The bridge body (_call_sync_or_async) adapts the sync/async surface via asyncio.run, so the sync Protocol annotations remain accurate from the bridge's perspective \u2014 it is the only consumer that ever touches these fields. * RunComponents.activity_client / connector_client now stay sync NoopActivityRuntimeClient / NoopConnectorClient in production by design. The production orchestrator uses the WF-IMPL-074 iter_calls yield-protocol which routes through the bridges above and never touches these slots; keeping them Noop guarantees no un-awaited coroutine leaks into the legacy ActivityStepHandler.execute() driver if a future caller invokes it directly with the production env in place. * Caller-supplied activity_client / connector_client kwargs still feed both slots symmetrically so existing test Fakes drive both the Step Coordinator and the bridges. * Update tests/test_providers.py to pin the new contract: outbound slots hold the production Dapr clients (or Noops), legacy sync slots stay Noop, and explicit overrides land on both slots.
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
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.
Closes #491.
Part of tracker #495.
Depends on #490 (WF-IMPL-079 outbound-activity bridges).
Summary
Wire the WF-IMPL-077
DaprActivityRuntimeClientand WF-IMPL-078DaprConnectorClientintoload_run_components()behind the design-lockedWF_ARM_ENDPOINT/WF_CONNECTOR_ENDPOINTenv knobs, plus a sharedWF_OUTBOUND_RPC_TIMEOUT_MSknob. A single lifespan-ownedhttpx.AsyncClientis shared across the publisher + ARM + Connector adapters (one socket pool per worker), and the WF-IMPL-079 bridges register against the resolved clients atruntime.start().Changes
src/services/workflow-service/src/custos_workflow/providers.py:ENV_ARM_APP_ID = "WF_ARM_ENDPOINT",ENV_CONNECTOR_APP_ID = "WF_CONNECTOR_ENDPOINT",ENV_OUTBOUND_RPC_TIMEOUT_MS = "WF_OUTBOUND_RPC_TIMEOUT_MS",DEFAULT_OUTBOUND_RPC_TIMEOUT_MS = 10_000._publisher_env_active(env)predicate factored out so the HTTP-client gate can short-circuit on any of three independent triggers._resolve_outbound_rpc_timeout_seconds(env)parses positive-int ms → seconds float. Bad values (non-integer, zero, negative) raiseValueErroreagerly so a misconfigured worker fails at startup rather than at first request._build_activity_client(env, http_client, timeout_seconds) -> ActivityRuntimeClient: returnsNoopActivityRuntimeClientwhen the env var is unset (preserves the WF-IMPL-043 sidecar-free dev / test path) andDaprActivityRuntimeClientcast to the sync Protocol when set._build_connector_clientmirrors.load_run_components()rewritten: gate the sharedhttpx.AsyncClientbuild on whether any Dapr consumer is active, then thread that single instance into all three adapters. Caller-supplied overrides (lifecycle_publisher,activity_client,connector_client) continue to short-circuit the env-driven build.WorkflowRuntimeis now constructed withactivity_runtime_client=andconnector_client=so the WF-IMPL-079 bridges register against the resolved adapters.src/services/workflow-service/tests/test_providers.py: 14 new tests across the four env-var branches (both unset, ARM only, Connector only, both set), pinningRunComponents.dapr_http_clientidentity through both adapters, the Noop fallback, the override short-circuit, the timeout-knob parsing happy / sad paths, and the end-to-end timeout-into-Dapr-adapter wiring.docs/developers/workflow-api.md: Configuration table extended with the three new rows (call out the single socket pool, the Noop fallback, and the startup-fail-fast on bad values).Quality gate
Local (macOS, Python 3.13):
ruff check --fix,ruff format --check,mypy src tests,pytest -q --hypothesis-seed=0— 1959 passed, 1 documented macOS-only flake (test_observability.py::test_module_imports_under_noop_providers, subprocess-coverage on the hidden-pth path; Linux CI is authoritative). Coverage 99.02% (floor 90%).