🐛 [cdk] Return correct type during 'check' if config does not match schema

[yfyf]

trafficstars

What

This fixes a bug where, upon config validation against the JSON schema failure, the run_cmd function would yield the connection status message in the form of a serialized JSON (i.e. str). This violated the output signature of Iterable[AirbyteMessage] and would produce unclear errors when run from the CLI.

Curiously, mypy did not catch this simple type error, though pyright did.

How

Yield the correct type upon exception.

Review guide

It's a one-line change, except for the test.

Added a small unit test that covers this scenario.

User Impact

When running check --config bad-config.json, it would throw up with:

{"type": "LOG", "log": {"level": "FATAL", "message": "'str' object has no attribute 'json'\nTraceback <..>}}
{"type": "TRACE", "trace": {"type": "ERROR", <..>}}

Now it produces a clear message:

 {"type": "CONNECTION_STATUS", "connectionStatus": {"status": "FAILED", "message": "Config validation error: 'foo' is a required property"}}

Can this PR be safely reverted and rolled back?

• [X] YES 💚
• [ ] NO ❌

[ellipsis-dev[bot]]

Your free trial has expired. To continue using Ellipsis, sign up at https://app.ellipsis.dev for $20/developer/month. If you have any questions, reach us at [email protected]

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 24, 2024 1:33pm

[CLAassistant]

All committers have signed the CLA.

[natikgadzhi]

Uh oh, we missed it. @yfyf thank you for the fix! Let me spin up CI on this and merge if everything looks fine.

[natikgadzhi]

@alafanechere will add CDK to allowed paths to trigger community CI and then merge this :D

[yfyf]

@alafanechere I'm sorry, but I can't dedicate more time to this at the moment. It is also a code maintenance change that isn't related to the bug fix.

On Fri, May 24, 2024, 16:42 Augustin @.***> wrote:

@.**** requested changes on this pull request.

Hey @yfyf https://github.com/yfyf , we run mypy on modified files in CI. Would you mind fixing the following error on the destination.py module?

airbyte-cdk/python/airbyte_cdk/destinations/destination.py:115: error: Function is missing a return type annotation [no-untyped-def]

— Reply to this email directly, view it on GitHub https://github.com/airbytehq/airbyte/pull/37398#pullrequestreview-2076974363, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFZNDVSO33HINCXQN24HTTZD47VZAVCNFSM6AAAAABGMY2W4WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANZWHE3TIMZWGM . You are receiving this because you were mentioned.Message ID: @.***>

[natikgadzhi]

Since this is not a net new mypy error and it's unrelated to the actual change, I'll merge this as is. Thank you for the contribution, @yfyf!