Lightweight export/import of configurations scoped by workspace Id #5598

ChristopheDuong · 2021-08-24T17:32:34Z

What

Describe what the change is solving
Closes #5124

How

Describe the solution

export only configurations related to a workspace id from the config database (not the job database)
import configurations into a workspace (tweaking configurations id as needed if any conflicts). This step is done in two API calls by going through a staging resource file first before actually importing.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions
Connector version bumped like described here

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing.
New Connector version released on Dockerhub by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed.

sherifnada

I think not having tests for these new methods/class makes me super nervous. Could we add those?

airbyte-api/src/main/openapi/config.yaml

sherifnada · 2021-08-25T06:14:09Z

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpExporter.java

+        .collect(Collectors.toList());
+    writeConfigsToArchive(parentFolder, ConfigSchema.SOURCE_CONNECTION.name(), sourceConnections.stream().map(Jsons::jsonNode));
+
+    final Map<UUID, StandardSourceDefinition> sourceDefinitionMap = new HashMap<>();


this method is formidable. can we break it up a little? are there any DRY opportunities here?

Yes, I just got the basic functionality working, I'll refactor parts and add tests now

sherifnada · 2021-08-25T06:23:05Z

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpExporter.java

+    return dump;
+  }
+
+  private void exportConfigsDatabase(Path parentFolder, UUID workspaceId) throws IOException, JsonValidationException, ConfigNotFoundException {


at a high level it feels like there should be an export(list<uuid> workspaceIds) instead of having two different implementations one scoped by workspace. Would that make sense here or am I missing something?

This export function scoped by workspace Id is exporting configurations related to a workspace and then "cascade" down to other configurations that are being referred/dependent on them.

The second export without scope is dumping everything regardless of links/relations between configurations and thus, will cover more configurations than this export(uuid) or a export(list<uuid>) function

jrhizor · 2021-08-25T14:26:09Z

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpImporter.java

+      final Stream<T> configs = readConfigsFromArchive(sourceRoot, configSchema);
+      if (!dryRun) {
+        switch (configSchema) {
+          case STANDARD_SOURCE_DEFINITION -> importStandardSourceDefinitionsIntoWorkspace(configs);


How are we going to handle this on cloud now?

I still need to introduce a wrapped version of this that changes the behavior in cloud

For the moment, i'm thinking about adding a different configDumpImporter that ignores all standard definitions for the short term (since we don't allow custom connectors yet)

https://github.com/airbytehq/airbyte-cloud/pull/178

tuliren · 2021-08-26T22:02:55Z

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpImporter.java

+      // 1. Unzip source
+      Archives.extractArchive(archive.toPath(), sourceRoot);
+
+      // TODO: Auto-migrate archive?


I have thought about the archive auto migration feature. It will be a medium size project. We need to write version-specific importer that can understand imports of different versions.

Issue here: #5682

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpExporter.java

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpImporter.java

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpExporter.java

tuliren · 2021-08-27T16:14:14Z

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpImporter.java

+    // We sort the directories because we want to process SOURCE_CONNECTION after
+    // STANDARD_SOURCE_DEFINITION and DESTINATION_CONNECTION after STANDARD_DESTINATION_DEFINITION
+    // so that we can identify which definitions should not be upgraded to the latest version
+    directories.sort(Comparator.reverseOrder());


Since we don't update the definition in this method, the sorting here seems unnecessary.

we update definitions when its a new definition to create.

This should be done before importing the connector as it checks that definitions exists first

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpImporter.java

…ter.java

ChristopheDuong added 3 commits August 24, 2021 19:29

Scope export/import by workspace id

ed57cbb

use writeConfig instead of replaceAllConfigs

dfa05b3

Split importIntoWorkspace into 2 API endpoints

04cd27a

github-actions bot added area/api Related to the api area/platform issues related to the platform area/documentation Improvements or additions to documentation labels Aug 24, 2021

ChristopheDuong requested review from tuliren and sherifnada August 24, 2021 17:38

sherifnada reviewed Aug 25, 2021

View reviewed changes

jrhizor reviewed Aug 25, 2021

View reviewed changes

ChristopheDuong added 3 commits August 25, 2021 17:54

Refactor code to make it DRY

0c5b926

Add test case

f4cbdf3

Add unit test for importer

95129e6

ChristopheDuong requested a review from sherifnada August 26, 2021 17:36

tuliren reviewed Aug 26, 2021

View reviewed changes

Apply changes from code review

9321587

ChristopheDuong requested a review from tuliren August 27, 2021 11:35

ChristopheDuong added 2 commits August 27, 2021 16:11

Expose archive handler constructor

c819378

Add some checks before persisting imported configs

110eec1

tuliren approved these changes Aug 27, 2021

View reviewed changes

ChristopheDuong commented Aug 27, 2021

View reviewed changes

airbyte-server/src/main/java/io/airbyte/server/ConfigDumpImporter.java Outdated Show resolved Hide resolved

Update airbyte-server/src/main/java/io/airbyte/server/ConfigDumpImpor…

2d12c9b

…ter.java

ChristopheDuong requested a review from jrhizor August 27, 2021 16:28

format code

82ccb7e

ChristopheDuong merged commit 314a747 into master Aug 30, 2021

ChristopheDuong deleted the chris/scope-export-import branch August 30, 2021 09:00

jrhizor mentioned this pull request Sep 3, 2021

Bump Airbyte version from 0.29.13-alpha to 0.29.14-alpha #5831

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lightweight export/import of configurations scoped by workspace Id #5598

Lightweight export/import of configurations scoped by workspace Id #5598

ChristopheDuong commented Aug 24, 2021 •

edited

Loading

sherifnada left a comment

sherifnada Aug 25, 2021

ChristopheDuong Aug 25, 2021

sherifnada Aug 25, 2021

ChristopheDuong Aug 25, 2021

jrhizor Aug 25, 2021

ChristopheDuong Aug 25, 2021

ChristopheDuong Aug 25, 2021

ChristopheDuong Aug 27, 2021

tuliren Aug 26, 2021

tuliren Aug 27, 2021

ChristopheDuong Aug 27, 2021

Lightweight export/import of configurations scoped by workspace Id #5598

Lightweight export/import of configurations scoped by workspace Id #5598

Conversation

ChristopheDuong commented Aug 24, 2021 • edited Loading

What

How

Recommended reading order

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

sherifnada left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopheDuong commented Aug 24, 2021 •

edited

Loading