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

Exclude connectors with unsupported protocol version from seed updates #19328

Merged
merged 5 commits into from
Nov 15, 2022

Conversation

gosusnp
Copy link
Contributor

@gosusnp gosusnp commented Nov 11, 2022

What

When apply connector updates from seeds, we should check that the connectors have supported protocol versions.

Relates to #15463

How

Add a filter on protocol version in ApplyDefinitionsHelper.

NOTE: there is currently some duplication, this is partly due to concurrent PRs. I will have a follow up PR to merge the duplicated parts and remove the deprecated constructors once we bumped cloud.

Recommended reading order

  1. ApplyDefinitionsHelper.java contains the main logic
  2. DefaultJobPersistence.java contains a helper to retrieve the current protocol version support range

@gosusnp gosusnp temporarily deployed to more-secrets November 11, 2022 00:59 Inactive
@octavia-squidington-iv octavia-squidington-iv added the area/platform issues related to the platform label Nov 14, 2022
@gosusnp gosusnp temporarily deployed to more-secrets November 14, 2022 19:39 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets November 14, 2022 19:53 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets November 14, 2022 21:44 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets November 14, 2022 22:13 Inactive
@gosusnp gosusnp temporarily deployed to more-secrets November 15, 2022 00:30 Inactive
@@ -73,13 +73,13 @@ public Optional<AirbyteProtocolVersionRange> validate(final boolean supportAutoU
return Optional.of(targetRange);
}

if (currentRange.equals(targetRange)) {
if (currentRange.isEmpty() || currentRange.get().equals(targetRange)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit; if (currentRangs.orElse(targetRange).equals(targetRange))

@gosusnp gosusnp merged commit c47890d into master Nov 15, 2022
@gosusnp gosusnp deleted the gosusnp/prevent-upgrading-out-of-range-connectors branch November 15, 2022 16:36
akashkulk pushed a commit that referenced this pull request Dec 2, 2022
#19328)

* Filter out connectors with unsupported protocol in ApplyDefinitionsHelper

* Format

* Remove code dupl
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants