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

🚀 Amazon SP: expand endpoint support #4863

Merged
merged 20 commits into from
Aug 6, 2021
Merged

🚀 Amazon SP: expand endpoint support #4863

merged 20 commits into from
Aug 6, 2021

Conversation

htrueman
Copy link
Contributor

@htrueman htrueman commented Jul 20, 2021 •
edited
Loading

What

Closes #4594

How

Add support for GET_MERCHANT_LISTINGS_ALL_DATA and GET_FBA_INVENTORY_AGED_DATA stream endpoints
Update connector to support airbyte-cdk.

Recommended reading order

  1. Schema files
  2. source.py
  3. streams.py
  4. the rest

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

Connector Generator checklist

  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • If adding a new generator, add it to the list of scaffold modules being tested
  • The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
  • Documentation which references the generator is updated as needed.

@github-actions github-actions bot added the area/connectors Connector related issues label Jul 20, 2021
@Zirochkaa
Copy link
Contributor

tests?

Zirochkaa
Zirochkaa approved these changes Jul 20, 2021
View reviewed changes
Copy link
Contributor

@Zirochkaa Zirochkaa left a comment

Choose a reason for hiding this comment

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

lgtm, just a few comments. Also, please run tests.

@htrueman htrueman requested review from Zirochkaa and keu July 22, 2021 12:03
@htrueman
Copy link
Contributor Author

htrueman commented Jul 22, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1056033312
❌ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1056033312

@htrueman
Copy link
Contributor Author

htrueman commented Jul 22, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1056074851
✅ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1056074851

@htrueman
Copy link
Contributor Author

htrueman commented Jul 22, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1056089928
✅ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1056089928

@Zirochkaa
Copy link
Contributor

Zirochkaa commented Jul 23, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1058703823
❌ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1058703823

keu
keu requested changes Jul 23, 2021
View reviewed changes
Copy link
Contributor

@keu keu left a comment

Choose a reason for hiding this comment

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

see my comments

...te-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/auth.py Outdated Show resolved Hide resolved
...te-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/auth.py Outdated Show resolved Hide resolved
...te-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/auth.py Outdated Show resolved Hide resolved
...te-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/auth.py Outdated Show resolved Hide resolved
...integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/streams.py Outdated Show resolved Hide resolved
...integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/streams.py Outdated Show resolved Hide resolved
...integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/streams.py Outdated Show resolved Hide resolved
@Zirochkaa
Copy link
Contributor

Zirochkaa commented Jul 23, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1059228056
❌ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1059228056

@htrueman
Copy link
Contributor Author

htrueman commented Jul 23, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1059863563
❌ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1059863563

@htrueman
Copy link
Contributor Author

htrueman commented Jul 23, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1059948211
❌ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1059948211

@htrueman
Copy link
Contributor Author

htrueman commented Jul 23, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1059994480
✅ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1059994480

@keu keu changed the title 🚀 Amazon sp expand endpoint support 🚀 Amazon SP expand endpoint support Jul 23, 2021
@htrueman htrueman requested a review from keu July 30, 2021 13:49
keu
keu approved these changes Aug 1, 2021
View reviewed changes
Copy link
Contributor

@keu keu left a comment

Choose a reason for hiding this comment

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

small comments

@htrueman
Typing added.
1a7d817 
Add _create_prepared_request docstring.
@htrueman
Copy link
Contributor Author

htrueman commented Aug 1, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1087086442
✅ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1087086442

Phlair
Phlair reviewed Aug 3, 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.

Good work on this. Main points of review:

  • Have you tested that the UI works as expected? I recently had to some messing around with what pydantic spits out to render properly in the UI.
  • Should update the connector's doc page .md to reflect new functionality.
  • Also need to version bump this as outlined here . Since this looks like it would likely break backwards compatibility this should be a minor version bump (e.g. x.x.x -> x.y.0). Also need to add this to doc changelog.

airbyte-integrations/connectors/source-amazon-seller-partner/requirements.txt Show resolved Hide resolved
...-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/source.py Outdated Show resolved Hide resolved
...-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/source.py Show resolved Hide resolved
...-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/source.py Show resolved Hide resolved
...-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/spec.json Outdated Show resolved Hide resolved
...-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/source.py Show resolved Hide resolved
sherifnada
sherifnada approved these changes Aug 4, 2021
View reviewed changes
...-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/source.py Outdated Show resolved Hide resolved
...-integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/source.py Show resolved Hide resolved
...integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/streams.py Show resolved Hide resolved
...integrations/connectors/source-amazon-seller-partner/source_amazon_seller_partner/streams.py
return {self.cursor_field: max(latest_benchmark, current_stream_state[self.cursor_field])}
return {self.cursor_field: latest_benchmark}

def _create_prepared_request(
Copy link
Contributor

Choose a reason for hiding this comment

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

can we create an issue to add to the CDK and an issue to update this once that issue is resolved? ideally we should never override an _ method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, here it is #5246

@sherifnada
Copy link
Contributor

clicked approve by accident, just meant to leave a few comments. We can merge once @Phlair 's comments are addressed

@htrueman
Update docs and spec
359c8f4
@htrueman
Merge remote-tracking branch 'origin/master' into htrueman/source-ama…
6ef8446 
…zon-sp-expand-endpoint-support

# Conflicts:
#	airbyte-config/init/src/main/resources/config/STANDARD_SOURCE_DEFINITION/e55879a8-0ef8-4557-abcf-ab34c53ec460.json
#	airbyte-config/init/src/main/resources/seed/source_definitions.yaml
@htrueman
Post merge code fixes
900fcb8
@htrueman
Copy link
Contributor Author

htrueman commented Aug 6, 2021 •
edited by github-actions bot
Loading

/publish connector=connectors/source-amazon-seller-partner

🕑 connectors/source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1105284830
✅ connectors/source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1105284830

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Aug 6, 2021
@jrhizor jrhizor temporarily deployed to more-secrets August 6, 2021 13:48 Inactive
@htrueman
Copy link
Contributor Author

htrueman commented Aug 6, 2021 •
edited by github-actions bot
Loading

/test connector=source-amazon-seller-partner

🕑 source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1105315703
✅ source-amazon-seller-partner https://github.com/airbytehq/airbyte/actions/runs/1105315703

@jrhizor jrhizor temporarily deployed to more-secrets August 6, 2021 14:00 Inactive
@htrueman htrueman merged commit ba322ac into master Aug 6, 2021
@htrueman htrueman deleted the htrueman/source-amazon-sp-expand-endpoint-support branch August 6, 2021 14:10
This was referenced Aug 6, 2021
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Phlair Phlair Phlair left review comments

@keu keu keu approved these changes

@sherifnada sherifnada sherifnada approved these changes

@Zirochkaa Zirochkaa Zirochkaa approved these changes

@yevhenii-ldv yevhenii-ldv Awaiting requested review from yevhenii-ldv

@ChristopheDuong ChristopheDuong Awaiting requested review from ChristopheDuong

@tuliren tuliren Awaiting requested review from tuliren

@davinchia davinchia Awaiting requested review from davinchia

Assignees
No one assigned
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/amazon-seller-partner connectors/sources-api
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🚀 Source Amazon SP API: Expand endpoint support
7 participants
@htrueman @Zirochkaa @sherifnada @keu @Phlair @jrhizor @igrankova