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 shopify source #1763

Closed
5 tasks
sherifnada opened this issue Jan 21, 2021 · 1 comment
Closed
5 tasks

Adopt connector best practices for shopify source #1763

sherifnada opened this issue Jan 21, 2021 · 1 comment
Assignees

Comments

@sherifnada
Copy link
Contributor

Tell us about the problem you're trying to solve

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

Since this connector is Singer based, for each of the below items please verify whether:

  • the singer tap already implements it
  • the singer tap does not implement it and would require forking the tap to implement
  • it's something we can implement easily on top of the Singer tap without forking it

If implementing any of the below items would require forking the Singer tap, please notify @sherifnada and move on to the next issue.

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 21, 2021
@yevhenii-ldv yevhenii-ldv self-assigned this Jan 22, 2021
@yevhenii-ldv
Copy link
Contributor

The estimate time to complete this task is 8 hours.

Checklist

  • Check connection verifies permissions upfront - 1h
  • Check connection failure emits actionable error messages. - 30m
  • If API connector: integration tests validate that every stream outputs data (you may need to seed the underlying API with fake data). - 4h to fill the account with fake data for all streams using API and test every stream.
  • If API connector: backs off on hitting rate limits (instead of just failing/exiting) - 30m on testing
  • If API connector: implements incremental sync (where possible -- not all streams will allow this) - 2h

yevhenii-ldv added a commit that referenced this issue Jan 25, 2021
sherifnada added a commit that referenced this issue Jan 28, 2021
Co-authored-by: Sherif Nada <snadalive@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants