SAT: Capture configuration updates from connectors' control messages #19979

alafanechere · 2022-12-01T14:53:20Z

What

1/2 of this issue.

#19428 introduced a new CDK feature: connectors can emit control messages to update their configuration.
The latest emitted configuration should be used on the next connector operations.

We need to make SAT capable of:

Reading control messages emitted by connectors under test
Storing updated configurations
Using the latest updated configurations in tests

Demo 👀

Loom video

How

~~Tweak source-bing-ads for testing: make its config observed and produce control messages on token refresh~~ <- reverted
Change ConnectorRunner.run to process control messages that are updating configuration:

airbyte/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/connector_runner.py

Line 116 in 234ba61

if (
Implement ConnectorRunner._persist_new_configuration to store new configurations to an updated_configurations folder (e.g. secrets/updated_configurations/config|{update-time}.json)

airbyte/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/connector_runner.py

Line 183 in f879c01

def _persist_new_configuration(self, new_configuration: dict, configuration_emitted_at: int) -> Optional[Path]:
Change the connector_configuration_path fixture to retrieve the latest updated configuration if any:

airbyte/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/conftest.py

Line 58 in 234ba61

def connector_config_path_fixture(inputs, base_path) -> Path:

🚨 User Impact 🚨

This should not impact existing SAT run as no connector is yet emitting CONTROL messages.

github-actions · 2022-12-01T14:54:53Z

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

Run integration tests
Bump connector or module version
Add changelog
Publish the new version

❌ Sources (64)

Connector	Version	Changelog	Publish
`source-airtable`	`0.1.3`	✅	✅
`source-amazon-ads`	`0.1.25`	✅	✅
`source-amazon-seller-partner`	`0.2.28`	✅	✅
`source-amazon-sqs`	`0.1.0`	✅	✅
`source-amplitude`	`0.1.17`	✅	✅
`source-appsflyer`	`0.1.0`	✅	✅
`source-asana`	`0.1.5`	✅	✅
`source-azure-table`	`0.1.3`	✅	✅
`source-braintree`	`0.1.3`	✅	✅
`source-cart`	`0.2.0`	✅	✅
`source-chargebee`	`0.1.16`	✅	✅
`source-commercetools`	`0.1.0`	✅	✅
`source-confluence`	`0.1.1`	✅	✅
`source-datadog`	`0.1.0`	✅	✅
`source-delighted`	`0.1.4`	✅	✅
`source-drift`	`0.2.5`	✅	✅
`source-facebook-marketing`	`0.2.76`	✅	✅
`source-facebook-pages`	`0.1.6`	✅	✅
`source-freshcaller`	`0.1.0`	✅	✅
`source-freshsales`	`0.1.2`	✅	✅
`source-freshservice`	`0.1.1`	✅	✅
`source-github`	`0.3.8`	✅	✅
`source-gitlab`	`0.1.8`	✅	✅
`source-google-ads`	`0.2.5`	✅	✅
`source-google-search-console`	`0.1.18`	✅	✅
`source-greenhouse`	`0.3.0`	✅	✅
`source-harvest`	`0.1.11`	✅	✅
`source-instagram`	`1.0.0`	✅	✅
`source-iterable`	`0.1.22`	✅	✅
`source-klaviyo`	`0.1.10`	✅	✅
`source-lemlist`	`0.1.1`	✅	✅
`source-lever-hiring`	`0.1.3`	✅	✅
`source-linnworks`	`0.1.5`	✅	✅
`source-mailchimp`	`0.3.0`	✅	✅
`source-mailgun`	`0.1.0`	✅	✅
`source-monday`	`0.1.4`	✅	✅
`source-notion`	`0.1.10`	✅	✅
`source-okta`	`0.1.13`	✅	✅
`source-onesignal`	`0.1.2`	✅	✅
`source-openweather`	`0.1.6`	✅	✅
`source-outreach`	`0.1.2`	✅	✅
`source-pardot`	`0.1.0`	✅	✅
`source-paystack`	`0.1.1`	✅	✅
`source-pinterest`	`0.1.9`	✅	✅
`source-pipedrive`	`0.1.13`	✅	✅
`source-plaid`	`0.3.2`	❌ (changelog missing)	✅
`source-posthog`	`0.1.7`	✅	✅
`source-prestashop`	`0.3.0`	✅	✅
`source-quickbooks-singer`	`0.1.5`	⚠ (doc not found)	✅
`source-recharge`	`0.2.4`	✅	✅
`source-retently`	`0.1.2`	✅	✅
`source-salesforce`	`1.0.27`	✅	✅
`source-salesloft`	`0.1.3`	✅	✅
`source-sendgrid`	`0.2.16`	✅	✅
`source-sentry`	`0.1.7`	✅	✅
`source-strava`	`0.1.2`	✅	✅
`source-surveymonkey`	`0.1.13`	✅	✅
`source-tplcentral`	`0.1.1`	✅	✅
`source-twilio`	`0.1.14`	✅	✅
`source-weatherstack`	`0.1.0`	✅	✅
`source-youtube-analytics`	`0.1.3`	✅	✅
`source-zendesk-sunshine`	`0.1.1`	✅	✅
`source-zendesk-talk`	`0.1.5`	✅	✅
`source-zenloop`	`0.1.3`	✅	✅

See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (0)

Connector	Version	Changelog	Publish

See "Actionable Items" below for how to resolve warnings and errors.

✅ Other Modules (0)

Actionable Items

(click to expand)

Category	Status	Actionable Item
Version	❌ mismatch	The version of the connector is different from its normal variant. Please bump the version of the connector.
	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
Changelog	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
	❌ changelog missing	There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish	⚠ not in seed	The connector is not in the seed file (e.g. `source_definitions.yaml`), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.
	❌ diff seed version	The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the `/publish` command to publish the latest version.

alafanechere · 2022-12-02T12:20:13Z

/test connector=connectors/source-bing-ads

🕑 connectors/source-bing-ads https://github.com/airbytehq/airbyte/actions/runs/3601629813
❌ connectors/source-bing-ads https://github.com/airbytehq/airbyte/actions/runs/3601629813
🐛 https://gradle.com/s/pyi5xyokdru6i

Build Failed

Test summary info:

Could not find result summary

alafanechere · 2022-12-02T12:20:40Z

~~TODO: Revert changes on source-bings-ads.~~ ✅

evantahler · 2022-12-02T16:36:44Z

Link your demo look video to the PR!

evantahler · 2022-12-02T16:37:14Z

~~I'll fixup source-plaid's changelog~~
Edit - I think I merged that fix in yesterday - should be good if you merge in master

alafanechere · 2022-12-02T16:41:42Z

Link your demo look video to the PR!

I think I already did 😄

sherifnada

some comments and questions but nothing blocking

sherifnada · 2022-12-02T23:00:24Z

airbyte-integrations/connectors/source-bing-ads/source_bing_ads/client.py

-        auth_creds = {
-            "client_id": client_id,
+    @property
+    def auth_creds(self):


nit: signatures

sherifnada · 2022-12-02T23:00:36Z

airbyte-integrations/connectors/source-bing-ads/source_bing_ads/client.py

            "redirection_uri": "",  # should be empty string
-            "client_secret": None,
-            "tenant": tenant_id,


do we not need tenant?

...e-integrations/bases/source-acceptance-test/source_acceptance_test/utils/connector_runner.py

sherifnada · 2022-12-02T23:03:51Z

...e-integrations/bases/source-acceptance-test/source_acceptance_test/utils/connector_runner.py

@@ -93,6 +95,7 @@ def call_read_with_state(self, config, catalog, state, **kwargs) -> List[Airbyte
        return output

    def run(self, cmd, config=None, state=None, catalog=None, raise_container_error: bool = True, **kwargs) -> Iterable[AirbyteMessage]:


does the calling context automatically get this new config? if yes can you clarify how in a comment?

All the tests using the ConnectorRunner are using the connector_config fixture (also function scoped) . The connector_config fixture is loading the config from the connector_config_path fixture. The connector_config_path fixture dynamically retrieves the latest configuration written to theupdated_configurations folder. So all tests using connector_config fixture are calling docker_runner methods with the "new" config.

I don't think clarifying the behavior here is a good idea as it's fixture related, not an internal logic of the runner. I wrote something here in the connector_config_path fixture.

...e-integrations/bases/source-acceptance-test/source_acceptance_test/utils/connector_runner.py

alafanechere · 2022-12-05T11:18:25Z

Thanks @sherifnada for your review. I answered your comments about SAT.
I'm not going to make additional changes to the bing ads connector change as I only quickly modified this connector for testing and will revert the changes made in this PR.

sherifnada · 2022-12-06T01:09:10Z

@alafanechere makes sense, thank you for the clarifications! LGTM :)

alafanechere · 2022-12-06T12:14:50Z

/publish connector=bases/source-acceptance-test auto-bump-version=false

🕑 Publishing the following connectors:
bases/source-acceptance-test
https://github.com/airbytehq/airbyte/actions/runs/3629414545

Connector	Did it publish?	Were definitions generated?
bases/source-acceptance-test	✅	❌

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

alafanechere added 2 commits December 1, 2022 15:51

use observed config in bing ads for testing

5f5948a

update local configurations and use most recent

b37d788

octavia-squidington-iv added area/connectors Connector related issues connectors/source/bing-ads labels Dec 1, 2022

alafanechere added 4 commits December 2, 2022 11:37

test connector runner changes

aebc8a5

test_connector_config_path_fixture

7a8ecde

make connector_configuration_path optional in ContainerRunner

234ba61

add docstrings

c251eb9

alafanechere marked this pull request as ready for review December 2, 2022 11:56

bump version

f879c01

alafanechere requested review from sherifnada and a team December 2, 2022 12:20

alafanechere added area/live-tests from/connector-ops labels Dec 2, 2022

Merge branch 'master' into augustin/sat/capture-config-messages

0cd81af

sherifnada approved these changes Dec 2, 2022

View reviewed changes

alafanechere mentioned this pull request Dec 5, 2022

ci-credentials: update GSM secrets with updated configuration values #20076

Merged

alafanechere added 2 commits December 6, 2022 10:58

gitingore updated_configurations

da3917e

revert changes on source-bing-ads

e1255a7

octavia-squidington-iv removed the area/connectors Connector related issues label Dec 6, 2022

alafanechere removed the connectors/source/bing-ads label Dec 6, 2022

alafanechere merged commit d28964c into master Dec 6, 2022

alafanechere deleted the augustin/sat/capture-config-messages branch December 6, 2022 14:23

This was referenced Dec 6, 2022

Bump Airbyte version from 0.40.23 to 0.40.24 #20161

Closed

Bump Airbyte version from 0.40.23 to 0.40.24 #20206

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAT: Capture configuration updates from connectors' control messages #19979

SAT: Capture configuration updates from connectors' control messages #19979

alafanechere commented Dec 1, 2022 •

edited

Loading

github-actions bot commented Dec 1, 2022 •

edited

Loading

❌ Sources (64)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

alafanechere commented Dec 2, 2022 •

edited by github-actions bot

Loading

alafanechere commented Dec 2, 2022 •

edited

Loading

evantahler commented Dec 2, 2022

evantahler commented Dec 2, 2022 •

edited

Loading

alafanechere commented Dec 2, 2022

sherifnada left a comment

sherifnada Dec 2, 2022

sherifnada Dec 2, 2022

sherifnada Dec 2, 2022

alafanechere Dec 5, 2022

alafanechere commented Dec 5, 2022

sherifnada commented Dec 6, 2022

alafanechere commented Dec 6, 2022 •

edited by github-actions bot

Loading

		@@ -93,6 +95,7 @@ def call_read_with_state(self, config, catalog, state, **kwargs) -> List[Airbyte
		return output

		def run(self, cmd, config=None, state=None, catalog=None, raise_container_error: bool = True, **kwargs) -> Iterable[AirbyteMessage]:

SAT: Capture configuration updates from connectors' control messages #19979

SAT: Capture configuration updates from connectors' control messages #19979

Conversation

alafanechere commented Dec 1, 2022 • edited Loading

What

Demo 👀

How

🚨 User Impact 🚨

github-actions bot commented Dec 1, 2022 • edited Loading

Affected Connector Report

❌ Sources (64)

✅ Destinations (0)

✅ Other Modules (0)

Actionable Items

alafanechere commented Dec 2, 2022 • edited by github-actions bot Loading

Build Failed

alafanechere commented Dec 2, 2022 • edited Loading

evantahler commented Dec 2, 2022

evantahler commented Dec 2, 2022 • edited Loading

alafanechere commented Dec 2, 2022

sherifnada left a comment

Choose a reason for hiding this comment

sherifnada Dec 2, 2022

Choose a reason for hiding this comment

sherifnada Dec 2, 2022

Choose a reason for hiding this comment

sherifnada Dec 2, 2022

Choose a reason for hiding this comment

alafanechere Dec 5, 2022

Choose a reason for hiding this comment

alafanechere commented Dec 5, 2022

sherifnada commented Dec 6, 2022

alafanechere commented Dec 6, 2022 • edited by github-actions bot Loading

alafanechere commented Dec 1, 2022 •

edited

Loading

github-actions bot commented Dec 1, 2022 •

edited

Loading

alafanechere commented Dec 2, 2022 •

edited by github-actions bot

Loading

alafanechere commented Dec 2, 2022 •

edited

Loading

evantahler commented Dec 2, 2022 •

edited

Loading

alafanechere commented Dec 6, 2022 •

edited by github-actions bot

Loading