Skip to content
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

use launchdarkly feature flagging for column selection #22577

Merged
merged 6 commits into from
Feb 10, 2023

Conversation

mfsiega-airbyte
Copy link
Contributor

@mfsiega-airbyte mfsiega-airbyte commented Feb 8, 2023

What

Use LaunchDarkly feature-flagging for column selection.

How

Check whether column selection should be enabled for the workspace against the feature flag client (LaunchDarkly).

if yes -> do it
if no -> fall back on pre-existing env variable configs

This PR will always return no for now, so this is effectively a no-op. Then we can update the rules in LaunchDarkly as we see fit. Once we're happy with those, we can stop reading (and eventually remove) the various environment variables that are in use today.

Testing

To validate this change, I did the following: (1) deployed to dev-3; (2) disabled column selection per the various old env variable flags; (3) enabled in LD; (4) verified via logs that column selection was being applied in the backend.

@octavia-squidington-iii octavia-squidington-iii added area/platform issues related to the platform area/worker Related to worker labels Feb 8, 2023
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 8, 2023 17:14 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 8, 2023 17:14 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 8, 2023 17:18 — with GitHub Actions Inactive
Copy link
Member

@colesnodgrass colesnodgrass left a comment

Choose a reason for hiding this comment

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

👍🏻 as long as syncInput.getWorkspaceId() doesn't return null

Copy link
Contributor

@josephkmh josephkmh left a comment

Choose a reason for hiding this comment

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

Looks good to me. Also the feature flagging looks much less complicated with the new client - nice!

@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 10, 2023 11:48 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 10, 2023 11:48 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023

Airbyte Code Coverage

File Coverage [4.21%]
ReplicationJobOrchestrator.java 11.01%
ReplicationActivityImpl.java 0.63%
Total Project Coverage 24.66%

@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 10, 2023 12:09 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 10, 2023 12:09 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 10, 2023 12:17 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 10, 2023 12:56 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 10, 2023 12:56 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 10, 2023 12:58 — with GitHub Actions Inactive
@mfsiega-airbyte
Copy link
Contributor Author

Some notes:

  • I planned to add unit test coverage here, but discovered that these files have no unit test suites whatsoever. We should change that, but I don't want to block column selection on it.
  • To validate this change, I did the following: (1) deployed to dev-3; (2) disabled column selection per the various old env variable flags; (3) enabled in LD; (4) verified via logs that column selection was being applied in the backend.

All in all I'm very confident that the change is safe, and also pretty confident that it works as intended, so I'll go ahead here.

@mfsiega-airbyte mfsiega-airbyte merged commit 8554b5b into master Feb 10, 2023
@mfsiega-airbyte mfsiega-airbyte deleted the msiega/use-launchdarkly-for-column-selection branch February 10, 2023 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform area/worker Related to worker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants