Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ Source Braze : Migrate to Manifest-only #47329

Merged
merged 19 commits into from
Jan 29, 2025
Merged

✨ Source Braze : Migrate to Manifest-only #47329

merged 19 commits into from
Jan 29, 2025

Conversation

topefolorunso
Copy link
Collaborator

@topefolorunso topefolorunso commented Oct 24, 2024 •
edited by ChristoGrab
Loading

What

Migrates source-braze to manifest-only format and resolves bugs in the custom components

How

  • Migrates connector to manifest-only format (ie no Python boilerplate)
  • Updates custom incremental component to conform to CDK expectations. Increases stream slice size by 1 day to be inclusive (as per Braze API documentation)
  • Updates custom record extractor to properly handle response object in decoder

@vercel Vercel
Copy link

vercel bot commented Oct 24, 2024 •
edited
Loading

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 Jan 28, 2025 3:21pm

@octavia-squidington-iv octavia-squidington-iv requested a review from a team October 24, 2024 00:55
@topefolorunso
Copy link
Collaborator Author

topefolorunso commented Oct 24, 2024 •
edited by github-actions bot
Loading

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (bf6054d)

@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Oct 24, 2024
@topefolorunso
Copy link
Collaborator Author

@ChristoGrab can you populate empty streams failing CAT tests

  • 'campaigns_analytics',
  • 'canvases_analytics',
  • 'cards_analytics',
  • 'events',
  • 'segments_analytics'

@topefolorunso
Copy link
Collaborator Author

topefolorunso commented Jan 12, 2025 •
edited by github-actions bot
Loading

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (39527ed)

@DanyloGL
Copy link
Collaborator

@topefolorunso I've checked sandbox, mentioned streams are populated. Also there is incremental test failed. Could you please take a look?

@natikgadzhi
Copy link
Contributor

This one is important to me, I would love to get this merged and happy to re-review in the coming days.

@natikgadzhi
Copy link
Contributor

Incremental tests are failing, but once we get them fixed, I'd love to get this into a rollout still.

@natikgadzhi
Copy link
Contributor

Thank you for picking this up!

@DanyloGL
Copy link
Collaborator

DanyloGL commented Jan 28, 2025 •
edited by github-actions bot
Loading

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (8d22a2b)

natikgadzhi
natikgadzhi approved these changes Jan 28, 2025
View reviewed changes
Copy link
Contributor

@natikgadzhi natikgadzhi left a comment

Choose a reason for hiding this comment

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

The regression test report actually looks good to me. There are no records in campaigns stream which is sus, we should check that stream additionally, but this is okay to do in a progressive rollout. /cc @DanyloGL

@DanyloGL
Copy link
Collaborator

The regression test report actually looks good to me. There are no records in campaigns stream which is sus, we should check that stream additionally, but this is okay to do in a progressive rollout. /cc @DanyloGL

Yes, but it strange as we have 2 connections on Cloud and both of them use Campaigns stream. Will run regression with one of them directly.

@natikgadzhi
Copy link
Contributor

@DanyloGL sounds good! Try to merge this on Wednesday so this doesn't go stale over the weekend.

@DanyloGL
Copy link
Collaborator

@DanyloGL sounds good! Try to merge this on Wednesday so this doesn't go stale over the weekend.

Both connections on cloud had no records for Campaigns stream( Merging and will monitor database

@DanyloGL DanyloGL merged commit ae23d5a into master Jan 29, 2025
34 checks passed
@DanyloGL DanyloGL deleted the tope/braze/migrate-manifest-only branch January 29, 2025 15:57
btkcodedev pushed a commit that referenced this pull request Feb 3, 2025
@topefolorunso @octavia-squidington-iii
@DanyloGL @natikgadzhi @ChristoGrab @btkcodedev
✨ Source Braze : Migrate to Manifest-only (#47329)
96c4b9b 
Co-authored-by: Octavia Squidington III <[email protected]>
Co-authored-by: Danylo Jablonski <[email protected]>
Co-authored-by: Natik Gadzhi <[email protected]>
Co-authored-by: ChristoGrab <[email protected]>
stephane-airbyte pushed a commit that referenced this pull request Feb 4, 2025
@topefolorunso @octavia-squidington-iii
@DanyloGL @natikgadzhi @ChristoGrab @stephane-airbyte
✨ Source Braze : Migrate to Manifest-only (#47329)
50eb780 
Co-authored-by: Octavia Squidington III <[email protected]>
Co-authored-by: Danylo Jablonski <[email protected]>
Co-authored-by: Natik Gadzhi <[email protected]>
Co-authored-by: ChristoGrab <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natikgadzhi natikgadzhi natikgadzhi approved these changes

@ChristoGrab ChristoGrab Awaiting requested review from ChristoGrab

Assignees
No one assigned
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/braze
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@topefolorunso @DanyloGL @natikgadzhi @octavia-squidington-iii @ChristoGrab