feat: skip config validation during discovery for declarative sources that don't use DynamicSchemaLoader

[devin-ai-integration]

Preface from AJ

This came up in the S.H.I.T. list and it's something @bnchrch and I have discussed a bit. It came up again today in Slack, raised as an inquiry from Michel. I decided to have Devin to a spike, and it looks like this could be close to shippable.

What changes:

1. Declaratives sources should be able to run --discover without a config, unless they use DynamicSchemaLoader, in which case, they will require config.
2. This only affects the discover verb. Other verbs are unchanged.

Prior art:

• Meltano similarly implemented something similar, here: https://gitlab.com/meltano/sdk/-/merge_requests/188

Devin-Created Summary: Allow Airbyte sources to run discovery unprivileged with DynamicSchemaLoader

This PR allows Airbyte sources to run discovery unprivileged if the source API doesn't need auth in order to provide the catalog info.

Implementation

1. Added automatic detection of DynamicSchemaLoader to skip config validation during discovery. When a source uses the dynamic schema feature, we assume that the schema endpoint might not require authentication and automatically skip config validation during discovery.

2. Modified the AirbyteEntrypoint to make the --config parameter optional for the discover command. This allows running discovery without providing a config file when the source doesn't require config validation.

This enables sources with dynamic schemas to provide catalog information without authentication when the schema endpoint doesn't require it.

Testing

Added unit tests to verify that:

• Sources using DynamicSchemaLoader skip config validation during discovery
• Sources not using DynamicSchemaLoader still require config validation
• The AirbyteEntrypoint correctly handles discovery without config when appropriate

Link to Devin run: https://app.devin.ai/sessions/e6aa19df336347919b6cabcff7143a1c
Requested by: Aaron ("AJ") Steers ([email protected])

[devin-ai-integration]

Original prompt from Aaron:

@Devin - We want to allow Airbyte sources to run discovery unprivileged if the source API doesn't need auth in order to provide the catalog info. Please try to update the declarative manifest implementation in the CDK to succeed in running `discover` even if no auth ("config") is provided by the user. Make sure you test up front that running discovery does indeed fail. Then try to update the connector to skip the check for config. We can use a flag in the source class (or another suitable location) to tell the implementation explicitly to require config or not during discovery.

[devin-ai-integration]

🤖 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 and CI monitoring

[aaronsteers]

I've done a few rounds of review and I think this is looking well. I'll open up now to other reviewers and mark ready for review.

[aaronsteers]

aside - Note: I corrected Devin's implementation to use generator comprehension instead of list comprehension, and Devin applies in these adjacent locations as well. I think this is a positive change, calling out though to explain why other code paths are touched.

More about generator comprehensions here: https://stackoverflow.com/a/47826

[aaronsteers]

/autofix

Auto-Fix Job Info

This job attempts to auto-fix any linting or formating issues. If any fixes are made,
those changes will be automatically committed and pushed back to the PR.

Note: This job can only be run by maintainers. On PRs from forks, this command requires
that the PR author has enabled the Allow edits from maintainers option.

PR auto-fix job started... Check job output.

✅ Changes applied successfully.

[aaronsteers]

I am moving this back to draft status and will dive deeper later when time permits.

Directionally, I think this is doable and makes sense, and most of the code here in the PR is correct, I believe. But I want to add some tests and make sure we're getting the failure behaviors we expect.

Will move future work to this PR with me as author:

• #467

Feel free to drop comments/suggestions here on this PR and I'll make sure to consider them in future work.

[michel-tricot]

the value and the default mentionned in the comment don't align.

[michel-tricot]

why is this changing?

[devin-ai-integration]

Closing due to inactivity for more than 7 days.

[devin-ai-integration]

Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it.