Skip to content

fix(declarative): fail fast on unmapped async job status - #1139

Draft
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787942778-async-unmapped-status-fail-fast
Draft

fix(declarative): fail fast on unmapped async job status#1139
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787942778-async-unmapped-status-fail-fast

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

An async-job stream whose status_mapping did not cover a status the API actually returns silently polled until polling_job_timeout expired instead of failing. AsyncHttpJobRepository._get_validated_job_status() raised a plain ValueError, and AsyncJobOrchestrator._is_breaking_exception() only breaks on AirbyteTracedException with FailureType.config_error, so the unmapped status was recorded as a non-breaking exception, the job stayed RUNNING, and the loop kept polling until the timeout, ending in a generic failure. This was reported from a Connector Builder test read where the async stream took the full timeout before failing.

Two changes:

  1. Runtime — fail fast at the boundary where the raw API status is first adapted:
-        raise ValueError(f"API status `{api_status}` is unknown. Contact the connector developer ...")
+        raise AirbyteTracedException(
+            message=f'Async job status "{api_status}" is not supported by the connector.',
+            internal_message=f"... missing from the connector's `status_mapping`, which declares: {sorted(self.status_mapping.keys())} ...",
+            failure_type=FailureType.config_error,
+        )

Because the failure type is config_error, the orchestrator now aborts running jobs and re-raises on the first polling response instead of accumulating a non-breaking exception. This also covers the "absent status" case: when the status extractor yields nothing, api_status is None, which is likewise unmapped.

  1. Manifest-parse time — _create_async_job_status_mapping() now rejects a status map with no terminal success status (nothing mapped to completed or skipped), which is a map under which a job can never finish.

Note on (2): the issue suggested validating that running is non-empty. An empty running list is legitimate — jobs are RUNNING from creation until a terminal status is reported, and after change (1) an unrecognized non-terminal status fails immediately anyway. A map with no completed/skipped entry, by contrast, can never complete, so that is the condition validated here.

Declarative-First Evaluation

No custom Python component involved; both changes are in shared CDK code (the async job repository and the declarative parser). No connector manifest changes.

Reproduction

No live reproduction of the originally reported connector was possible (no API credentials available), so this is reproduced at the CDK level instead: the existing test_given_unknown_status_when_update_jobs_status_then_raise_error mock-HTTP test showed the old plain ValueError, and the new orchestrator test reproduces the polling behavior — with the old exception type the orchestrator keeps polling; with a config_error-typed exception it raises on the first update and aborts the job.

Test Coverage

  • unit_tests/sources/declarative/requesters/test_http_job_repository.py — unmapped status (invalid_status) and missing status field now assert AirbyteTracedException with failure_type == config_error, and that the message names the offending status.
  • unit_tests/sources/declarative/async_job/test_job_orchestrator.py — new regression test: a config_error AirbyteTracedException from update_jobs_status propagates out of create_and_get_completed_partitions() on the first iteration (update_jobs_status called exactly once, i.e. no polling until timeout) and running jobs are aborted.
  • unit_tests/sources/declarative/parsers/test_model_to_component_factory.py — an AsyncRetriever with completed: [] and no skipped now fails at component creation.

Commands run locally:

poetry run pytest unit_tests/ -x -q   # 4366 passed, 3 skipped
poetry run ruff format . && poetry run ruff check .

Breaking change evaluation

Not a breaking change per the connector breaking-change checklist: no schema, primary key, cursor, spec, stream, data-scope, or state-format change. This is an error-classification change in the CDK — a sync that previously failed after burning polling_job_timeout now fails immediately with a clearer message. The parse-time validation rejects only status maps under which async jobs could never complete. This repo derives its version from semantic-release tooling, so no manual version bump or changelog edit applies.

Resolves https://github.com/airbytehq/airbyte-internal-issues/issues/17032:

Requested by Patrick Nilan (@pnilan) on that issue.

Link to Devin session: https://app.devin.ai/sessions/39981653ad284a888e6f44c423481291
Open in Devin Desktop: https://app.devin.ai/desktop/session/39981653ad284a888e6f44c423481291?variant=devin

Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

Copy link
Copy Markdown

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1787942778-async-unmapped-status-fail-fast#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1787942778-async-unmapped-status-fail-fast

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /autofix - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test - Runs connector tests with the updated CDK
  • /prerelease - Triggers a prerelease publish with default arguments
  • /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
  • /poe <command> - Runs any poe command in the CDK environment
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves declarative async-job behavior in the Python CDK by failing fast when the API returns an unmapped or missing job status, preventing unnecessary polling until polling_job_timeout and surfacing a clearer connector/manifest configuration error.

Changes:

  • Raise an AirbyteTracedException with FailureType.config_error when an async job status is unmapped (or missing), so the orchestrator aborts running jobs immediately.
  • Add manifest parse-time validation to reject async status mappings that never map any API status to a terminal success (completed or skipped).
  • Update and add unit tests covering the new runtime exception behavior and the new parse-time validation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
airbyte_cdk/sources/declarative/requesters/http_job_repository.py Converts unmapped/missing API status into a config_error traced exception to stop polling immediately.
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py Adds validation requiring at least one terminal success status mapping (completed/skipped).
unit_tests/sources/declarative/requesters/test_http_job_repository.py Updates tests to assert AirbyteTracedException(config_error) for unknown and missing status cases.
unit_tests/sources/declarative/async_job/test_job_orchestrator.py Adds regression test ensuring config errors from status updates abort running jobs without repeated polling.
unit_tests/sources/declarative/parsers/test_model_to_component_factory.py Adds test ensuring invalid async status mappings fail during component creation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown

PyTest Results (Fast)

4 366 tests  +3   4 354 ✅ +3   7m 55s ⏱️ - 1m 39s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 60464ff. ± Comparison against base commit 7c96f9d.

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

4 369 tests  +3   4 357 ✅ +3   13m 55s ⏱️ +17s
    1 suites ±0      12 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 60464ff. ± Comparison against base commit 7c96f9d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants