Add missing prefix and icon fields in migration v0.20.0 #3503

tuliren · 2021-05-20T08:45:43Z

What

Two new fields prefix and icon were added in the v0.20.0 config schema. However, they were not specified in the relevant migration file.
Consequently, when users run any migration after v0.20.0, the migration runner will consider the input configs as invalid since they contain the two new fields undefined in the schema defined in migration resource, which is the source of truth for schema validation during migration.

Click to expand sample logs

2021-05-19 23:12:51 INFO i.a.m.MigrationRunner(parse):78 - {} - args: [--input, /config/airbyte_archive.tar.gz, --output, /Users/lirentu/Downloads/new_airbyte_archive.tar.gz]
2021-05-19 23:12:51 INFO i.a.m.MigrationRunner(run):50 - {} - Unpacking tarball
2021-05-19 23:12:51 INFO i.a.m.MigrationRunner(run):67 - {} - Running migrations...
2021-05-19 23:12:51 INFO i.a.m.MigrationRunner(run):68 - {} - MigrateConfig{inputPath=/tmp/airbyte_migrate7703568884416841818/uncompressed, outputPath=/tmp/airbyte_migrate7703568884416841818/output, targetVersion='null'}
2021-05-19 23:12:51 INFO i.a.m.Migrate(run):88 - {} - Starting migrations. Current version: 0.22.3-alpha, Target version: null
2021-05-19 23:12:51 INFO i.a.m.Migrate(run):112 - {} - Migrating from version: 0.22.0-alpha to version 0.23.0-alpha.
Exception in thread "main" java.lang.IllegalArgumentException: Input data schema does not match declared input schema.
	at io.airbyte.migrate.Migrate.lambda$runMigration$1(Migrate.java:139)
	at java.base/java.util.stream.ReferencePipeline$11$1.accept(ReferencePipeline.java:441)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:133)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497)
	at io.airbyte.migrate.migrations.MigrationV0_23_0.migrate(MigrationV0_23_0.java:94)
	at io.airbyte.migrate.MigrateWithMetadata.migrate(MigrateWithMetadata.java:52)
	at io.airbyte.migrate.Migrate.runMigration(Migrate.java:149)
	at io.airbyte.migrate.Migrate.run(Migrate.java:113)
	at io.airbyte.migrate.MigrationRunner.run(MigrationRunner.java:70)
	at io.airbyte.migrate.MigrationRunner.main(MigrationRunner.java:108)
Caused by: io.airbyte.validation.json.JsonValidationException: json schema validation failed.
errors: $.icon: is not defined in the schema and the schema does not allow additional properties
schema:
{
  "$schema" : "http://json-schema.org/draft-07/schema#",
  "$id" : "https://github.com/airbytehq/airbyte/blob/master/airbyte-config/models/src/main/resources/types/StandardDestinationDefinition.yaml",
  "title" : "StandardDestinationDefinition",
  "description" : "describes a destination",
  "type" : "object",
  "required" : [ "destinationDefinitionId", "name", "dockerRepository", "dockerImageTag", "documentationUrl" ],
  "additionalProperties" : false,
  "properties" : {
    "destinationDefinitionId" : {
      "type" : "string",
      "format" : "uuid"
    },
    "name" : {
      "type" : "string"
    },
    "dockerRepository" : {
      "type" : "string"
    },
    "dockerImageTag" : {
      "type" : "string"
    },
    "documentationUrl" : {
      "type" : "string"
    }
  }
}
object:
{
  "destinationDefinitionId" : "f7a7d195-377f-cf5b-70a5-be6b819019dc",
  "name" : "Redshift",
  "dockerRepository" : "airbyte/destination-redshift",
  "dockerImageTag" : "0.3.7",
  "documentationUrl" : "https://docs.airbyte.io/integrations/destinations/redshift",
  "icon" : "redshift.svg"
}
	at io.airbyte.validation.json.JsonSchemaValidator.ensure(JsonSchemaValidator.java:88)
	at io.airbyte.migrate.Migrate.lambda$runMigration$1(Migrate.java:137)
	... 15 more

How

This PR fixes this issue by adding the updated schemas to migration 0.20.0.

Questions

The same updated schemas (standard sync, source definition, and destination definition) also exist in migration resource for v0.23.0. I think they can be removed, since the schema change actually happened in 0.20.0.
The resource directory for migration 0.20.0 also contains a few other updated schemas (notification, notification type, and slack notification config). But these files are not referenced in migration 0.20.0. Is this expected? I think they should either be referenced there, or removed. @ChristopheDuong, can you confirm?

Pre-merge Checklist

Run integration tests
Publish Docker images

Recommended reading order

MigrationV0_20_0.java
The rest

ChristopheDuong · 2021-05-20T08:56:24Z

The resource directory for migration 0.20.0 also contains a few other updated schemas (notification, notification type, and slack notification config). But these files are not referenced in migration 0.20.0. Is this expected? I think they should either be referenced there, or removed. @ChristopheDuong, can you confirm?

They are "referenced" from the workspace schemas as they are nested in the workspace configuration object, so no I don't think they should be removed or you would run in similar errors on future migrations if notifications related fields are starting to be populated in user's workspace configs

airbyte-migration/README.md

jrhizor

👍 Looks good. Just to double check, this works end to end with the sample archive you were working with?

tuliren · 2021-05-20T21:42:11Z

👍 Looks good. Just to double check, this works end to end with the sample archive you were working with?

Yes.

With this PR alone, I was able to migration my v0.22.0 configs to v0.23.0.

With my other PRs, local migration from v0.22.0 to v0.24.0 worked.

Click to see logs

2021-05-20 21:40:32 INFO i.a.m.MigrationRunner(parse):78 - {} - args: [--input, /Users/lirentu/Downloads/airbyte_archive_3.tar.gz, --output, /Users/lirentu/Downloads/new_airbyte_archive_3.tar.gz]
2021-05-20 21:40:32 INFO i.a.m.MigrationRunner(run):50 - {} - Unpacking tarball
2021-05-20 21:40:32 INFO i.a.m.MigrationRunner(run):67 - {} - Running migrations...
2021-05-20 21:40:32 INFO i.a.m.MigrationRunner(run):68 - {} - MigrateConfig{inputPath=/tmp/airbyte_migrate9363732575843362322/uncompressed, outputPath=/tmp/airbyte_migrate9363732575843362322/output, targetVersion='null'}
2021-05-20 21:40:32 INFO i.a.m.Migrate(run):88 - {} - Starting migrations. Current version: 0.22.3-alpha, Target version: null
2021-05-20 21:40:32 INFO i.a.m.Migrate(run):112 - {} - Migrating from version: 0.22.0-alpha to version 0.23.0-alpha.
2021-05-20 21:40:33 WARN c.n.s.JsonMetaSchema(newValidator):338 - {} - Unknown keyword existingJavaType - you should define your own Meta Schema. If the keyword is irrelevant for validation, just use a NonValidationKeyword
2021-05-20 21:40:39 INFO i.a.m.Migrate(run):112 - {} - Migrating from version: 0.23.0-alpha to version 0.24.0-alpha.
2021-05-20 21:40:41 INFO i.a.m.m.MigrationV0_24_0(lambda$migrate$2):118 - {} - Schedule added to standard sync config for connection 440af383-7aa4-491c-ae54-c665463a94f7 (manual: false, schedule: {"timeUnit":"minutes","units":5})
2021-05-20 21:40:45 INFO i.a.m.Migrate(run):123 - {} - Migrations complete. Now on version: null
2021-05-20 21:40:45 INFO i.a.m.MigrationRunner(run):74 - {} - Migration output written to /Users/lirentu/Downloads/new_airbyte_archive_3.tar.gz

mrahul17 · 2021-05-22T13:31:48Z

Hi @tuliren I was facing the same issue, so I built an image locally with your commit and it worked.
Any chance this will be released soon? I am planning to migrate my instance from 0.20.0-alpha to latest.
The upgrading doc mentions the command as

docker run --rm -v <path to directory containing downloaded airbyte_archive.tar.gz>:/config airbyte/migration:<version you are upgrading to> --\
  --input /config/airbyte_archive.tar.gz\
  --output <path to where migrated archive will be written (should end in .tar.gz)>\
  [ --target-version <version you are migrating to or empty for latest> ]

Is it fine if I use a local image for running the migration but upgrade to the latest available version (0.23.0-alpha) ?

tuliren · 2021-05-22T20:43:45Z

Hey @light94,

Sorry about the inconvenience.

Any chance this will be released soon?

Yes, we will try to publish a new release soon. @jrhizor, what's our cadence for release? Can we do it this coming week?

Is it fine if I use a local image for running the migration but upgrade to the latest available version (0.23.0-alpha) ?

Yes. That should work. Just remember to specify --target-version 0.23.0-alpha. This is necessary because we have made new config changes in migration for 0.24.0-alpha. If you run migration from the local image to the latest version, it will upgrade the configs to 0.24.0-alpha, which has not been released yet, and are not compatible with 0.23.0-alpha.

mrahul17 · 2021-05-23T06:32:51Z

Thanks @tuliren , I ended up doing the migration using my dev image and it worked perfectly.

tuliren added 2 commits May 20, 2021 01:33

Add updated schema to migration 20

26c15af

Add readme

8cd84c1

tuliren requested review from jrhizor, ChristopheDuong and cgardens May 20, 2021 08:45

auto-assign bot requested review from masonwheeler and michel-tricot May 20, 2021 08:45

Format files

eb29eb4

tuliren removed request for michel-tricot and masonwheeler May 20, 2021 08:48

ChristopheDuong reviewed May 20, 2021

View reviewed changes

airbyte-migration/README.md Outdated Show resolved Hide resolved

Update readme

80551ff

jrhizor approved these changes May 20, 2021

View reviewed changes

tuliren mentioned this pull request May 20, 2021

add test that migration output schema same as source schema #3356

Merged

tuliren merged commit a31d6fb into master May 20, 2021

tuliren deleted the liren/add-missing-fields-in-migration branch May 20, 2021 21:45

tuliren added this to the Core - 2021-05-21 milestone May 22, 2021

tuliren self-assigned this May 22, 2021

tuliren removed this from the Core - 2021-05-21 milestone May 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing prefix and icon fields in migration v0.20.0 #3503

Add missing prefix and icon fields in migration v0.20.0 #3503

tuliren commented May 20, 2021 •

edited

Loading

ChristopheDuong commented May 20, 2021 •

edited

Loading

jrhizor left a comment

tuliren commented May 20, 2021 •

edited

Loading

mrahul17 commented May 22, 2021 •

edited

Loading

tuliren commented May 22, 2021

mrahul17 commented May 23, 2021

Add missing prefix and icon fields in migration v0.20.0 #3503

Add missing prefix and icon fields in migration v0.20.0 #3503

Conversation

tuliren commented May 20, 2021 • edited Loading

What

How

Questions

Pre-merge Checklist

Recommended reading order

ChristopheDuong commented May 20, 2021 • edited Loading

jrhizor left a comment

Choose a reason for hiding this comment

tuliren commented May 20, 2021 • edited Loading

mrahul17 commented May 22, 2021 • edited Loading

tuliren commented May 22, 2021

mrahul17 commented May 23, 2021

tuliren commented May 20, 2021 •

edited

Loading

ChristopheDuong commented May 20, 2021 •

edited

Loading

tuliren commented May 20, 2021 •

edited

Loading

mrahul17 commented May 22, 2021 •

edited

Loading