publish spec as part of connector publish script #5994

cgardens · 2021-09-11T20:37:18Z

Closes #5266

What

This PR is the second half of the project that was stared in this PR: Add scheduler client that pulls from bucket cache #5605
Our goal is to publish to a cache the spec for a connector when we release a new version of it. This cache can then be used by the app to pull specs without having to run a docker container. The previous PR handled setting up reading from the cache (and falling back on running the spec job if there was a cache miss). This PR handles writing to the cache when we publish a connector.
It helps to add screenshots if it affects the frontend.

How

In the manage.sh script add logic to push to our GCS bucket cache.
Note: I have sanity checked that this actually works locally.

Pre-merge Checklist

resolve open question below

Open Question

I'm not entirely sure of the most ergonomic way to handle the service account key that is needed to make this script work. See in-line comment below.

cgardens · 2021-09-11T20:38:02Z

...heduler/client/src/main/java/io/airbyte/scheduler/client/BucketSpecCacheSchedulerClient.java

@@ -90,15 +90,19 @@ public BucketSpecCacheSchedulerClient(final SynchronousSchedulerClient client, f



all changes in this file are just adding debug log statements.

cgardens · 2021-09-11T20:40:46Z

tools/integrations/manage.sh

+  # publish spec to cache. do so, by running get spec locally and then pushing it to gcs.
+  local tmp_spec_file; tmp_spec_file=$(mktemp)
+  docker run --rm "$versioned_image" spec | jq .spec > "$tmp_spec_file"
+  gcloud auth activate-service-account --key-file /Users/charles/Downloads/prod-ab-cloud-proj-bdb658ebbe5a.json


so this line is obviously wrong (since it mentions the FS on my local machine). As far as I can tell this script gets called from 2 places: 1. from people's local machines 2. from GH actions. I think my inclination is to add another argument for cmd_publish where the user has to publish the path to the service account key. @sherifnada does this feel like a reasonable way of going about it? It's a credential so it has to add some developer friction, but I'm trying to keep it as light as possible.

I guess it should be designed so that if someone doesn't pass the cred but their gsutil is already authed into a user that can access the bucket then it should still work as well.

seems reasonable to me to limit publishing new connector versions to only Github

kk. well it's possible in both places now :D

sherifnada · 2021-09-11T20:40:18Z

tools/integrations/manage.sh

+  # publish spec to cache. do so, by running get spec locally and then pushing it to gcs.
+  local tmp_spec_file; tmp_spec_file=$(mktemp)
+  docker run --rm "$versioned_image" spec | jq .spec > "$tmp_spec_file"
+  gcloud auth activate-service-account --key-file /Users/charles/Downloads/prod-ab-cloud-proj-bdb658ebbe5a.json


we should inject this key in CI and pull from an env variable
There generally shouldn't be a strong need to publish from a local machine as you can already skip tests from CI is needed

kk. made it so it was possible to so locally if needed. happens by default in the publish command on GH

cgardens · 2021-09-11T20:42:10Z

tools/integrations/manage.sh

+
+  # publish spec to cache. do so, by running get spec locally and then pushing it to gcs.
+  local tmp_spec_file; tmp_spec_file=$(mktemp)
+  docker run --rm "$versioned_image" spec | jq .spec > "$tmp_spec_file"


@sherifnada this will break if spec returns anything but a single airbyte message of type spec. do you have any sense of how bad of an assumption that is. i guess we can protect against this by getting a little fancier with jq.

I recommend filtering to the right spec message. It shouldn't be that much harder to filter to only spec messages right? (and there should definitely only be one of those)

cool. added new jq logic. it handles:

filtering non valid json

picking the first spec if multiple are returned

failing if no specs are returned

davinchia

Generally looks good.

1 small comment.

cgardens · 2021-09-13T22:18:33Z