Skip to content

bug(Source facebook-marketing): upgrade cdk to filter invalid fields - #41958

Merged
Aldo Gonzalez (aldogonzalez8) merged 3 commits into
masterfrom
aldogonzalez8/upgrad_cdk_to_fix_invalid_fields
Jul 15, 2024
Merged

Aldo Gonzalez (aldogonzalez8) merged 3 commits into
masterfrom
aldogonzalez8/upgrad_cdk_to_fix_invalid_fields

Conversation

@aldogonzalez8

Copy link
Copy Markdown
Contributor

What

We have seen a recent problem with configured catalog having deprecated fields in configuration. We now have a fix for this in CDK.

How

Upgrade CDK version to get fix on invalid fields.

User Impact

After unpinned old facebook-marketing version to now use the latest users should not see anymore this error.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@vercel

vercel Bot commented Jul 15, 2024 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 15, 2024 5:55pm

@octavia-squidington-iii Octavia Squidington III (octavia-squidington-iii) added the area/documentation Improvements or additions to documentation label Jul 15, 2024
@aldogonzalez8 Aldo Gonzalez (aldogonzalez8) changed the title Source facebook-marketing: upgrade cdk to filter invalid fields bug(Source facebook-marketing): upgrade cdk to filter invalid fields Jul 15, 2024
@aldogonzalez8
Aldo Gonzalez (aldogonzalez8) requested a review from a team July 15, 2024 17:52
[tool.poetry.dependencies]
python = "^3.9,<3.12"
airbyte-cdk = "^2.3.0"
airbyte-cdk = "^3.5.0"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is a pretty big bump. Let's make sure to test with live-tests

@aldogonzalez8 Aldo Gonzalez (aldogonzalez8) Jul 15, 2024 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Live tests are ongoing...

airbyte-ci connectors --name=source-facebook-marketing build --architecture=linux/arm64

poetry run pytest src/live_tests/regression_tests --connector-image=airbyte/source-facebook-marketing --pr-url=https://github.com/airbytehq/airbyte/pull/41958 --connection-id=81d16659-48bf-44f3-ad98-17f4561e225e

Will let you know results

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changing connection id as the other was not available for live tests (EU)

poetry run pytest src/live_tests/regression_tests --connector-image=airbyte/source-facebook-marketing --pr-url=https://github.com/airbytehq/airbyte/pull/41958 --connection-id=8d7631c6-edd4-4fe6-86ac-7d7efa3261f3

@aldogonzalez8 Aldo Gonzalez (aldogonzalez8) Jul 15, 2024 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Connection with errors link

Note: there is a problem currently with requests count and url in live tests that are not reflected in facebook-marketing

image

image

image

I can see the logging we added in the target logs on my local:
image

{"level": "WARN", "message": "Stream ads_insights_action_type: the following fields are deprecated and cannot be synced. {'conversion_lead_rate', 'cost_per_conversion_lead'}. Refresh the connection's source schema to resolve this warning.", "stack_trace": null}

{"level": "WARN", "message": "Stream ads_insights: the following fields are deprecated and cannot be synced. {'conversion_lead_rate', 'cost_per_conversion_lead'}. Refresh the connection's source schema to resolve this warning.", "stack_trace": null}

@aldogonzalez8 Aldo Gonzalez (aldogonzalez8) Jul 15, 2024 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did more testing on a different connection. It was not reported to have problems but wanted to check another case to grow confidence in this solution cc Alexandre Girard (@girarda)

Connection

image

image

Record count is consistent:
image

Also no errors found in airbyte messages of target:
image

Note: I couldn't find our warning for "the following fields are deprecated..." but this would be expected if there were no problems with the configured catalog; I just wanted to check that a healthy running connection would continue healthy.

This branch was successfully deployed

1 active deployment
Preview — 808789a9 Deployed Jul 15, 2024 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/facebook-marketing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants