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

🎉 New Source: Snapchat Marketing API #4843

Merged
merged 19 commits into from
Jul 28, 2021
Merged

🎉 New Source: Snapchat Marketing API #4843

merged 19 commits into from
Jul 28, 2021

Conversation

TymoshokDmytro
Copy link
Contributor

@TymoshokDmytro TymoshokDmytro commented Jul 19, 2021 •
edited
Loading

What

#3916 New Source: Snapchat Marketing API

How

Implement the Snapchat Marketing API

Recommended reading order

airbyte-integrations/connectors/source-snapchat-marketing

Pre-merge Checklist

Expand the checklist which is relevant for this PR.

Connector checklist

  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • Secrets are annotated with airbyte_secret in the connector's spec
  • Credentials added to Github CI if needed and not already present. instructions for injecting secrets into CI.
  • Unit & integration tests added as appropriate (and are passing)
    • Community members: please provide proof of this succeeding locally e.g: screenshot or copy-paste acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • /test connector=connectors/<name> command as documented here is passing.
    • Community members can skip this, Airbyters will run this for you.
  • Code reviews completed
  • Documentation updated
    • README.md
    • docs/SUMMARY.md if it's a new connector
    • Created or updated reference docs in docs/integrations/<source or destination>/<name>.
    • Changelog in the appropriate page in docs/integrations/.... See changelog example
    • docs/integrations/README.md contains a reference to the new connector
    • Build status added to build page
  • Build is successful
  • Connector version bumped like described here
  • New Connector version released on Dockerhub by running the /publish command described here
  • No major blockers
  • PR merged into master branch
  • Follow up tickets have been created
  • Associated tickets have been closed & stakeholders notified

@TymoshokDmytro TymoshokDmytro self-assigned this Jul 19, 2021
@TymoshokDmytro TymoshokDmytro linked an issue Jul 19, 2021 that may be closed by this pull request
New Source: Snapchat Marketing API #3916
Closed
@github-actions github-actions bot added the area/connectors Connector related issues label Jul 19, 2021
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Jul 26, 2021
@TymoshokDmytro TymoshokDmytro marked this pull request as ready for review July 26, 2021 20:47
@TymoshokDmytro
Copy link
Contributor Author

TymoshokDmytro commented Jul 27, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-snapchat-marketing

🕑 connectors/source-snapchat-marketing https://github.com/airbytehq/airbyte/actions/runs/1071022977
✅ connectors/source-snapchat-marketing https://github.com/airbytehq/airbyte/actions/runs/1071022977

@sherifnada sherifnada requested review from Phlair and removed request for tuliren July 27, 2021 18:40
tuliren
tuliren approved these changes Jul 27, 2021
View reviewed changes
Copy link
Contributor

@tuliren tuliren left a comment

Choose a reason for hiding this comment

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

Thanks for writing all the docstring!

...grations/connectors/source-snapchat-marketing/integration_tests/test_schemas/adaccounts.json Outdated Show resolved Hide resolved
...grations/connectors/source-snapchat-marketing/integration_tests/test_schemas/adaccounts.json Outdated Show resolved Hide resolved
Phlair
Phlair reviewed Jul 28, 2021
View reviewed changes
Copy link
Contributor

@Phlair Phlair left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Awesome stuff, nice handling of the dependent streams ensuring full refresh on dependencies so we don't miss IDs and caching results for performance.

I didn't comment on all of them but if you could make sure all the .json files have newlines at end that would be great.

See my other comments for points, happy to discuss further. I think my main sticking point is adding a catch in case response_item_name is incorrect (could be on our side or API changed it etc.)

airbyte-integrations/connectors/source-snapchat-marketing/integration_tests/abnormal_state.json Show resolved Hide resolved
...-integrations/connectors/source-snapchat-marketing/integration_tests/configured_catalog.json Outdated Show resolved Hide resolved
...-integrations/connectors/source-snapchat-marketing/source_snapchat_marketing/schemas/TODO.md Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-snapchat-marketing/source_snapchat_marketing/source.py Show resolved Hide resolved
airbyte-integrations/connectors/source-snapchat-marketing/source_snapchat_marketing/source.py Show resolved Hide resolved
airbyte-integrations/connectors/source-snapchat-marketing/source_snapchat_marketing/source.py Show resolved Hide resolved
airbyte-integrations/connectors/source-snapchat-marketing/source_snapchat_marketing/source.py Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-snapchat-marketing/unit_tests/unit_test.py Outdated Show resolved Hide resolved
docs/integrations/sources/snapchat-marketing.md Outdated Show resolved Hide resolved
@TymoshokDmytro
Copy link
Contributor Author

TymoshokDmytro commented Jul 28, 2021 •
edited by github-actions bot
Loading

/test connector=connectors/source-snapchat-marketing

🕑 connectors/source-snapchat-marketing https://github.com/airbytehq/airbyte/actions/runs/1075803685
✅ connectors/source-snapchat-marketing https://github.com/airbytehq/airbyte/actions/runs/1075803685

Phlair
Phlair approved these changes Jul 28, 2021
View reviewed changes
Copy link
Contributor

@Phlair Phlair left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for adding the check!

@TymoshokDmytro
Copy link
Contributor Author

TymoshokDmytro commented Jul 28, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-snapchat-marketing

🕑 connectors/source-snapchat-marketing https://github.com/airbytehq/airbyte/actions/runs/1076056350
✅ connectors/source-snapchat-marketing https://github.com/airbytehq/airbyte/actions/runs/1076056350

@TymoshokDmytro TymoshokDmytro merged commit 82867c0 into master Jul 28, 2021
@TymoshokDmytro TymoshokDmytro deleted the dtymoshok/#3916_New_Snapchat_Source branch July 28, 2021 19:50
@inesmarcal inesmarcal mentioned this pull request Mar 30, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tuliren tuliren tuliren approved these changes

@midavadim midavadim midavadim approved these changes

@antixar antixar antixar approved these changes

@Phlair Phlair Phlair approved these changes

@lazebnyi lazebnyi lazebnyi approved these changes

@annalvova05 annalvova05 Awaiting requested review from annalvova05

@bazarnov bazarnov Awaiting requested review from bazarnov

@yaroslav-dudar yaroslav-dudar Awaiting requested review from yaroslav-dudar

@sherifnada sherifnada Awaiting requested review from sherifnada

@davinchia davinchia Awaiting requested review from davinchia

Assignees

@TymoshokDmytro TymoshokDmytro

Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New Source: Snapchat Marketing API
6 participants
@TymoshokDmytro @tuliren @midavadim @antixar @Phlair @lazebnyi