Separate connector upgrade from import #5965
Conversation
| importConfigsFromArchive(sourceRoot, seedPersistence, false); | ||
| // 4. Import Configs and update connector definitions | ||
| importConfigsFromArchive(sourceRoot, false); | ||
| configRepository.loadData(seedPersistence); |
There was a problem hiding this comment.
Maybe I'm missing something, but doesn't loadData need to handle the upgrade checks that we were handling in importConfigsFromArchive before to ensure we aren't overwriting source and destination definitions?
There was a problem hiding this comment.
Yes, DatabaseConfigPersistence#loadData does avoid updating a connector if it is used. The FileSystemConfigPersistence#loadData overwrites everything, but it is not used. Let me change it to throw an exception.
These existing tests ensure that we don’t update a connector definition if it is in use:
https://github.com/airbytehq/airbyte/blob/master/airbyte-config/persistence/src/test/java/io/airbyte/config/persistence/DatabaseConfigPersistenceLoadDataTest.java
https://github.com/airbytehq/airbyte/blob/master/airbyte-server/src/test/java/io/airbyte/server/handlers/ArchiveHandlerTest.java#L163
|
The failed auto migration test is being worked on in #5984. |
What
importConfigsFromArchivemethod. This makes the code complicated and buggy (see thread).importConfigsFromArchivemethod, and instead do that with the newloadDatamethod separately.How
loadDatamethod after the configs are imported.Recommended reading order
ConfigDumpImporter.java