Skip to content

(source-facebook-marketing): update to CDK v7#66570

Closed
pnilan wants to merge 7 commits intomasterfrom
pnilan/source-facebook-marketing/cdk7
Closed

(source-facebook-marketing): update to CDK v7#66570
pnilan wants to merge 7 commits intomasterfrom
pnilan/source-facebook-marketing/cdk7

Conversation

@pnilan
Copy link
Contributor

@pnilan pnilan commented Sep 22, 2025

What

  • Updates to CDK v7
  • Migrates connector to datetime/AirbyteDatetime from pendulum.

Review guide

  1. source.py
  2. utils.py
  3. api.py
  4. common.py
  5. base_streams.py
  6. async_job.py
  7. streams.py
  8. base_insight_streams.py
  9. tests

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@github-actions
Copy link
Contributor

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

Helpful Resources

PR Slash Commands

Airbyte Maintainers (that's you!) can execute the following slash commands on your PR:

  • /format-fix - Fixes most formatting issues.
  • /update-connector-cdk-version connector=<CONNECTOR_NAME> - Updates the specified connector to the latest CDK version.
    Example: /update-connector-cdk-version connector=destination-bigquery
  • /bump-version - Bumps connector versions.
    • You can specify a custom changelog by passing changelog. Example: /bump-version changelog="My cool update"
    • Leaving the changelog arg blank will auto-populate the changelog from the PR title.
  • /run-cat-tests - Runs legacy CAT tests (Connector Acceptance Tests)
  • /build-connector-images - Builds and publishes a pre-release docker image for the modified connector(s).
  • /poe connector source-example lock - Run the Poe lock task on the source-example connector, committing the results back to the branch.
  • /poe source example lock - Alias for /poe connector source-example lock.
  • /poe source example use-cdk-branch my/branch - Pin the source-example CDK reference to the branch name specified.
  • /poe source example use-cdk-latest - Update the source-example CDK dependency to the latest available version.

📝 Edit this welcome message.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2025

source-facebook-marketing Connector Test Results

261 tests   257 ✅  14s ⏱️
  2 suites    4 💤
  2 files      0 ❌

Results for commit 3ba283e.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2025

Deploy preview for airbyte-docs ready!

✅ Preview
https://airbyte-docs-gdx2azhid-airbyte-growth.vercel.app

Built with commit 3ba283e.
This pull request is being automatically deployed with vercel-action

@pnilan pnilan requested review from a team, brianjlai and maxi297 September 22, 2025 22:32
Copy link
Contributor

@maxi297 maxi297 left a comment

Choose a reason for hiding this comment

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

Have we run a couple of regression tests to validate that the behavior is the same?

@pnilan pnilan requested a review from maxi297 September 24, 2025 17:53
@pnilan pnilan assigned tolik0 and unassigned tolik0 Sep 24, 2025
@pnilan pnilan requested review from a team and tolik0 September 24, 2025 17:53
Copy link
Contributor

@maxi297 maxi297 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@aaronsteers aaronsteers left a comment

Choose a reason for hiding this comment

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

I read through the implementation and this all looks good to me. Nice work, @pnilan. 👍

@aaronsteers
Copy link
Collaborator

This prerelease-check failure looks like a false-positive:

❌ Pre-release CDK version detected.
{"version": "^7", "dependency_type": "version", "is_prerelease": true}
📝 Before merging your PR, remember to run `poe use-cdk-latest` to re-pin to the latest production CDK version.

@aaronsteers
Copy link
Collaborator

aaronsteers commented Sep 24, 2025

@pnilan - Looks like the pre-release check regex here expects "^7.0.0" in place of "^7".

I can create a PR to update that regex, but for this PR, I suggest just swapping with a 3-part version to silence the warning. Wdyt?

UPDATE: PR here:

@pnilan
Copy link
Contributor Author

pnilan commented Sep 25, 2025

Closing as it's been wrapped into @tolik0 's PR

@pnilan pnilan closed this Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants