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: Yotpo [Low code CDK] #25145

Closed
wants to merge 0 commits into from
Closed

🎉 New Source: Yotpo [Low code CDK] #25145

wants to merge 0 commits into from

Conversation

btkcodedev
Copy link
Collaborator

What

Developing new connector for the source: Yotpo

Resolves:
Yotpo

How

Developed using (Configuration Based Source) low-code CDK

Recommended reading order

  1. spec.yaml
  2. manifest.yaml
  3. schemas/*

Tests

Integration & Acceptance Full Test Results

image

🚨 User Impact 🚨

  • No breaking changes, Just addition of new source
New Connector: Source Yotpo

Community member or Airbyter

  • Community member? Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and 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.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • PR name follows PR naming conventions

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation bounty community connectors/source/yotpo labels Apr 13, 2023
@btkcodedev btkcodedev changed the title 🎉 New Source: Yotpo 🎉 New Source: Yotpo [Low code CDK] Apr 13, 2023
@btkcodedev
Copy link
Collaborator Author

@RealChrisSean
All acceptance tests are passing in local (Attached screenshot as description)
Please assign it for review :)

@btkcodedev
Copy link
Collaborator Author

btkcodedev commented Apr 13, 2023 •
edited
Loading

PS for reviewers:
Please ping via slack for getting sandbox account credentials and secrets for testing.

All available streams for free plan was successfully implemented, other streams sadly needs pricing plan
Slack username: btkcodedev :)

@marcosmarxm
Copy link
Member

Thanks for the contribution @btkcodedev right now our team is reviewing GA connector changes and can take a while to get to your contribution. Thanks for your patience.

marcosmarxm
marcosmarxm requested changes Apr 17, 2023
View reviewed changes
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

Left some comments please check them @btkcodedev

airbyte-integrations/connectors/source-yotpo/source_yotpo/spec.yaml Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-yotpo/integration_tests/sample_config.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-yotpo/source_yotpo/schemas/reviews.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-yotpo/source_yotpo/spec.yaml Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-yotpo/source_yotpo/manifest.yaml Outdated
type: DpathExtractor
field_path: ["reviews"]
paginator:
type: NoPagination
Copy link
Member

Choose a reason for hiding this comment

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

Please add more data into the stream to validate the paginator.

Copy link
Member

Choose a reason for hiding this comment

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

@btkcodedev don't resolve conversation if they weren't solved.

airbyte-integrations/connectors/source-yotpo/source_yotpo/manifest.yaml Outdated
Comment on lines 96 to 103
type: HttpRequester
url_base: "https://api.yotpo.com"
http_method: "GET"
request_parameters:
utoken: "{{ config['access_token'] }}"
authenticator:
type: BearerAuthenticator
api_token: "{{ config['access_token'] }}"
Copy link
Member

Choose a reason for hiding this comment

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

Create a YoptoRequester to reuse for all streams.

Copy link
Member

Choose a reason for hiding this comment

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

What is the difference from reviews, reviews bottomline and top reviews? What data each one doesn't have compared to the other? Why not only use reviews?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually 3 streams differ from each other, top_reviews only results 5 star rating, reviews has incremental reads and other one reads review with no ratings, Streams will be useful for the users

Copy link
Member

Choose a reason for hiding this comment

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

But top_reviews and reviews bottomline aren't only a subset of reviews?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is correct :) Removed.

airbyte-integrations/connectors/source-yotpo/source_yotpo/schemas/webhooks.json Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-yotpo/source_yotpo/manifest.yaml Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-yotpo/source_yotpo/manifest.yaml Outdated Show resolved Hide resolved
airbyte-integrations/connectors/source-yotpo/source_yotpo/manifest.yaml Outdated Show resolved Hide resolved
@btkcodedev
Copy link
Collaborator Author

All comments resolved :)

@btkcodedev btkcodedev requested a review from marcosmarxm April 18, 2023 10:42
@btkcodedev
Copy link
Collaborator Author

Test passed

image

marcosmarxm
marcosmarxm requested changes Apr 20, 2023
View reviewed changes
airbyte-integrations/connectors/source-yotpo/source_yotpo/manifest.yaml Outdated
field_name: "per_page"
pagination_strategy:
type: "PageIncrement"
page_size: 5
Copy link
Member

Choose a reason for hiding this comment

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

Where did you find this information in Yopto docs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was incorrect, Corrected :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved

airbyte-integrations/connectors/source-yotpo/source_yotpo/manifest.yaml Outdated Show resolved Hide resolved
marcosmarxm
marcosmarxm requested changes Apr 20, 2023
View reviewed changes
airbyte-integrations/connectors/source-yotpo/source_yotpo/manifest.yaml Outdated
type: DpathExtractor
field_path: ["reviews"]
paginator:
type: NoPagination
Copy link
Member

Choose a reason for hiding this comment

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

@btkcodedev don't resolve conversation if they weren't solved.

@btkcodedev
Copy link
Collaborator Author

btkcodedev commented Apr 23, 2023 •
edited
Loading

Well well, @marcosmarxm, I've checked the paginator as per https://apidocs.yotpo.com/reference/guidelines, and it is working, But it breaks the incremental read with the following error
image

Thus rolled back to only one review and thus connector passed the acceptance tests

@btkcodedev
Copy link
Collaborator Author

image

Test results

@btkcodedev btkcodedev requested a review from marcosmarxm April 23, 2023 18:56
@btkcodedev
Copy link
Collaborator Author

@marcosmarxm Little Bump Here, cause the deadline is by 27th🙂

@marcosmarxm
Copy link
Member

@btkcodedev can you solve the conflict please?

@CLAassistant
Copy link

CLAassistant commented Apr 26, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@btkcodedev btkcodedev closed this Apr 26, 2023
@octavia-squidington-iii octavia-squidington-iii removed area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Apr 26, 2023
@btkcodedev
Copy link
Collaborator Author

@marcosmarxm Pushing from #25532

@btkcodedev btkcodedev mentioned this pull request Apr 26, 2023
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcosmarxm marcosmarxm Awaiting requested review from marcosmarxm

Assignees
No one assigned
Labels
bounty community connectors/source/yotpo hackthon-developer-week new-source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@btkcodedev @marcosmarxm @CLAassistant @octavia-squidington-iii