airbyte icon indicating copy to clipboard operation
airbyte copied to clipboard

airbyte-cron: update connector definitions from remote

Open pedroslopez opened this issue 3 years ago • 1 comments
trafficstars

What

closes https://github.com/airbytehq/airbyte-cloud/issues/2542

Adds functionality to update source and destination definitions from the remote catalog at runtime, on an interval.

This builds on the remote definitions provider introduced in #16018

How

  • Set up airbyte-cron to connect to the configs db by following in airbyte-workers footsteps
  • Add a new DefinitionsUpdater task for airbyte-cron scheduled to run every 30 secs
  • Move definitions update logic to new helper ApplyDefinitionsHelper to re-use between bootloader/cron and standardize between oss/cloud. This will replace cloud's ApplySeedsToDefinitions class.
  • Use deployment mode env var to determine if we should overwrite everything or be more granular in how the update is performed
  • Allow using UPDATE_DEFINITIONS_CRON_ENABLED env var to turn on / off these updates

Also fixed a couple of things while in here:

  • When updating definitions for OSS, fix crash that occurs if there's a non-semver docker image tag in the db

Recommended reading order

  1. ApplyDefinitionsHelper
  2. DefinitionsUpdater

🚨 User Impact 🚨

This will be off by default on OSS, so no impact.

pedroslopez avatar Sep 08 '22 14:09 pedroslopez

@pedroslopez LGTM. We should include @benmoriceau on this review as well, as he is working on a PR for airbyte-cron that may conflict with this one.

jdpgrailsdev avatar Sep 12 '22 13:09 jdpgrailsdev

Hi @gosusnp / @jdpgrailsdev could I get confirmation that this is ok to merge via a review/approval? I have merged in the latest changes from master that remove the airbyte-worker dependency and my changes here are not affected.

Note that I'm defining my own DatabaseBeanFactory here which is why I don't need to depend on airbyte-worker, but I think de-duplicating this can be tackled as part of the micronaut tech debt epic that was being discussed.

pedroslopez avatar Sep 15 '22 15:09 pedroslopez

@pedroslopez I'm fine with merging this. We will want to do some cleanup later to avoid duplicating the configuration factory bits, but that shouldn't hold this up if we have confirmed that the airbyte-workers bit has been removed. I'll defer to @benmoriceau and @gosusnp to make sure that there isn't something in here that would impact the work they are doing to extract some classes from airbyte-workers for use in this service (I don't believe that this is an issue, but worth double checking).

jdpgrailsdev avatar Sep 15 '22 15:09 jdpgrailsdev