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

feat: generate full connector catalog json #18562

Merged
merged 12 commits into from
Nov 3, 2022

Conversation

pedroslopez
Copy link
Contributor

@pedroslopez pedroslopez commented Oct 27, 2022

What

Adds auto-generated oss_catalog.json so that we can upload it to GCS and use it from a RemoteDefinitionsProvider in cloud catalog generation.

This is a part of the cloud catalog generation decoupling tech spec https://docs.google.com/document/d/1_UP8JBVZwo2EDPxXmbvAMg0NQzlS30zF6IjUmsuLM_w/edit#

closes https://github.com/airbytehq/airbyte-cloud/pull/3162

How

🚨 User Impact 🚨

Once this is merged, as part of the build we will generate the file. This happens before the "no files changed" CI check occurs, which means that if any connector update is merged after this is in but does not have these changes that generate updates to the file, the master build will fail.

@pedroslopez pedroslopez temporarily deployed to more-secrets October 27, 2022 18:25 Inactive
@pedroslopez pedroslopez temporarily deployed to more-secrets October 27, 2022 18:39 Inactive
@pedroslopez pedroslopez marked this pull request as ready for review October 27, 2022 18:45
@pedroslopez pedroslopez changed the title Pedroslopez/oss combo catalog gen feat: generate full connector catalog json Oct 27, 2022
@pedroslopez pedroslopez requested review from alafanechere, evantahler and a team October 27, 2022 18:46
@@ -7,6 +7,7 @@ dependencies {

implementation project(':airbyte-commons')
implementation project(':airbyte-commons-cli')
implementation project(':airbyte-commons-docker')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added so that we can use the DockerUtils to get the tagged docker image name

@@ -121,6 +121,7 @@ def createSpotlessTarget = { pattern ->
'secrets',
'charts', // Helm charts often have injected template strings that will fail general linting. Helm linting is done separately.
'resources/seed/*_specs.yaml', // Do not remove - this is necessary to prevent diffs in our github workflows, as the file diff check runs between the Format step and the Build step, the latter of which generates the file.
'resources/seed/*_catalog.json', // Do not remove - this is also necessary to prevent diffs in our github workflows
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to ignore this from the format to avoid failures when CI checks if everything has been formatted. Same has already been happening above for the generated spec yamls.

@@ -32,4 +33,18 @@ task generateConnectorSpecsMask(type: JavaExec, dependsOn: generateSeedConnector

project(":airbyte-config:init").tasks.processResources.dependsOn(generateConnectorSpecsMask)

task generateCombinedConnectorCatalog(type: JavaExec, dependsOn: generateSeedConnectorSpecs) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New gradle task - you can either call gw :airbyte-config:init:generateCombinedConnectorCatalog or the usual gw :airbyte-config:init:processResources since it's added as a dependency

@pedroslopez pedroslopez temporarily deployed to more-secrets October 27, 2022 19:24 Inactive
Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

Should airbyte-config/init/src/main/resources/seed/oss_catalog.json be gitignored as it's always going to be auto-generated, and we want to discourage it's local use? This sort-of feels like all the java /build files to me...

@pedroslopez
Copy link
Contributor Author

pedroslopez commented Oct 28, 2022

Should airbyte-config/init/src/main/resources/seed/oss_catalog.json be gitignored as it's always going to be auto-generated, and we want to discourage it's local use? This sort-of feels like all the java /build files to me...

Oh yeah that's interesting - I had left these in the repo (cloud_catalog.json as well on the cloud side) mainly to see what was in there and have a track record of the changes / when they were introduced. But maybe we could just use the bucket itself for this instead of checking into git? Versioning is enabled on the bucket so we do have a history on that side.

Ignored and removed!

@pedroslopez pedroslopez temporarily deployed to more-secrets October 28, 2022 21:09 Inactive
@pedroslopez pedroslopez temporarily deployed to more-secrets November 2, 2022 23:24 Inactive
@pedroslopez pedroslopez merged commit bf43f62 into master Nov 3, 2022
@pedroslopez pedroslopez deleted the pedroslopez/oss-combo-catalog-gen branch November 3, 2022 00:16
letiescanciano added a commit that referenced this pull request Nov 3, 2022
* master: (38 commits)
  New Source: Gridly (#18342)
  🎉 New Source: Alpha Vantage (#18320)
  ci_integration_test.sh: cut GITHUB_STEP_SUMMARY (#18895)
  🎉 New Source: Datadog [python cdk] (#18150)
  Hide Reject all button in consent dialog (#18596)
  feat: add doc url to track event (#18690)
  fix: install java in oss catalog deploy action (#18887)
  [CI] Speed up check_images_exist (#18873)
  Extract open API (#18879)
  Remove unused interfaces (#18880)
  add action for deploying oss connector catalog to GCS (#18633)
  feat: generate full connector catalog json (#18562)
  Add unsupported_protocol_version column to Connection (#18876)
  Extract OAuth API (#18818)
  update images to have non-transparent background (#18874)
  DiscoverSchema endpoints calculates diff and breaking change (#18571)
  Validate protocol version on connector update (#18639)
  Bmoric/extract notification api (#18812)
  Show version and changelog status for affected connectors (#18845)
  Bmoric/extract logs api (#18621)
  ...
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.

3 participants