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.

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

adopt connector best practices for FB Marketing source #1566

Closed
2 of 5 tasks
sherifnada opened this issue Jan 7, 2021 · 4 comments
Closed
2 of 5 tasks

adopt connector best practices for FB Marketing source #1566

sherifnada opened this issue Jan 7, 2021 · 4 comments
Assignees
Labels
area/connectors Connector related issues type/enhancement New feature or request
Milestone

Comments

@sherifnada
Copy link
Contributor

sherifnada commented Jan 7, 2021

Tell us about the problem you're trying to solve

We'd like to follow the published connector development best practices on this connector.

Checklist

  • Check connection verifies permissions upfront
  • Check connection failure emits actionable error messages. This will probably require you to think about the failure modes that can occur (e.g: timeout, permission not granted, token does not exist, etc.. -- make sure that you add custom integration tests to verify each of these failure modes, or unit tests if you can mock the HTTP requests involved)
  • If API connector: integration tests validate that every stream outputs data (you may need to seed the underlying API with fake data). This will establish a baseline confidence that the connector works correctly on every data stream. If this is not possible, please reach out to @sherifnada with the reason to discuss how to move forward.
  • If API connector: backs off on hitting rate limits (instead of just failing/exiting)
  • If API connector: implements incremental sync (where possible -- not all streams will allow this)
@sherifnada sherifnada added type/enhancement New feature or request area/connectors Connector related issues zazmic labels Jan 7, 2021
@sherifnada sherifnada added this to the v0.12.0 milestone Jan 11, 2021
@eugene-kulak
Copy link
Contributor

eugene-kulak commented Jan 14, 2021

The estimation:

  • Check connection verifies permissions upfront - 30m
  • Check connection failure emits actionable error messages. - 1h
  • Check if integration tests can be run for every stream - 1h
  • If API connector: backs off on hitting rate limits (instead of just failing/exiting) - 30m

Dev Total: 2h

@sherifnada
Copy link
Contributor Author

Thanks @eugene-kulak this looks great. Let's make sure we implement it in the right facebook connector 😜

@sherifnada
Copy link
Contributor Author

(the new connector introduced in #1552

@eugene-kulak
Copy link
Contributor

#1699 overlapped with this ticket and has all job done there

@cgardens cgardens modified the milestones: v0.12.0, Launch 1.0 Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues type/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants