-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bulk-cdk: add feature flag environments #46692
bulk-cdk: add feature flag environments #46692
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
794fecc
to
137d716
Compare
@johnny-schmidt you already did something for this I think? at least destination-dev-null respects cloud vs oss mode |
Interesting! I searched and found it in the test fixtures, the substring to search for is |
137d716
to
5610f26
Compare
airbyte-cdk/bulk/core/base/src/main/kotlin/io/airbyte/cdk/command/FeatureFlag.kt
Outdated
Show resolved
Hide resolved
This is ready for review, the only failing checks are connnector version bumps. |
Sorry, I missed this earlier. Micronaut automagically propagates env variables as properties. You get I'm not sure what this PR accomplishes on top of that other than to limit the list of supported vars? However, you can also get that by specifying includes/excludes when building the ApplicationContext. |
Limiting the supported vars is not the goal per se but there is value in having a clear catalog of which env vars are meaningful. The main purpose of this PR is to facilitate activating feature flags in regular junit tests which are not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pardon the delay. I had to confer with the team to make sure it wasn't breaking anything test-related
No worries! Like I said it's not urgent and I only just got the build to pass. Thanks for taking a look. |
What
Fixes https://github.com/airbytehq/airbyte-internal-issues/issues/9704
The Bulk CDK needs to support the
DEPLOYMENT_MODE
environment variable and the associatedCLOUD
feature flag.This PR adds support for this and makes source-mysql-v2 adopt it.
How
As far as I can tell, none of the other feature flags from the legacy CDK matter for connectors.
In any case, the Bulk CDK now has a Micronaut Factory for injecting
Set<FeatureFlag>
whereFeatureFlag
is an enum and the injected value is the set of active feature flags. This is used in source-mysql-v2 to cause CHECK to fail when a config doesn't have enough encryption.Review guide
Commit by commit
User Impact
None
Can this PR be safely reverted and rolled back?