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

remove secrets migration code #7437

Merged
merged 1 commit into from
Oct 28, 2021
Merged

Conversation

jrhizor
Copy link
Contributor

@jrhizor jrhizor commented Oct 27, 2021

This was used for a one-time set of migrations. Doesn't seem worth maintaining after the migration, especially since it would require modifications with @lmossman's upcoming changes. We can always look at the old code and #7150 as a reference.

@jrhizor jrhizor temporarily deployed to more-secrets October 27, 2021 23:39 Inactive
Copy link
Contributor

@lmossman lmossman left a comment

Choose a reason for hiding this comment

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

LGTM

Wanted to confirm my understanding about this; Charles said that the idea with these one-time migrations is that we will require users on old enough versions to need to upgrade to a specific version before upgrading to the latest one, which will allow us to delete the code for these one-time migrations (e.g. this PR).

Does this PR mean that the "specific version" that they will need to upgrade to needs to be a version from before this PR is merged, so that they have the SecretsMigration class to use?

Copy link
Contributor

@lmossman lmossman left a comment

Choose a reason for hiding this comment

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

As discussed in weekly planning, we should hold off on merging this until we are sure that we have everything we need for the intermediate migration

@jrhizor
Copy link
Contributor Author

jrhizor commented Oct 28, 2021

This is a script to introduce a specific feature that is only used (as far as we know) for Cloud. New instances configured with the secret store feature enabled would work.

Exporting from your old instance + importing your configs into the new instance is really how you should handle it. It's more complicated for cloud.

This is separate from Charles's discussion about one time breaking migrations for all users, so we should be safe to merge this.

@jrhizor jrhizor merged commit a020618 into master Oct 28, 2021
@jrhizor jrhizor deleted the jrhizor/remove-migration-code branch October 28, 2021 17:40
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants