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

🎉 Source bigcommerce: add brands and categories streams #20518

Conversation

PhilipCorr
Copy link
Contributor

@PhilipCorr PhilipCorr commented Dec 15, 2022

What

Add two new streams. One for brands and one for categories.

How

The endpoints for these are described here:
https://developer.bigcommerce.com/api-reference/c2610608c20c8-get-all-brands
https://developer.bigcommerce.com/api-reference/9cc3a53863922-get-all-categories

They are both very similar in that the url matches:

https://api.bigcommerce.com/stores/{store_hash}/v3/catalog/{data_field}

where the data field is either brands or categories.
Neither of these endpoints have a date field that can be used for incremental syncing.
Instead, similar to the existing Pages source, they have an id field.
This is why we need to override the read_records method and explicitly call the filter_records_newer_than_state method.

Recommended reading order

Start with source.py.
Everything should make sense after that as the streams are reasonably standard.

🚨 User Impact 🚨

No breaking changes. This just adds two new streams which the user can opt into pulling if they want.

Pre-merge Checklist

Updating a connector

Community member or Airbyter

  • 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. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Tests

Unit

There are no unit tests for the bigcommerce connector.

Integration

There are no integration tests for the bigcommerce connector.

Acceptance

===========================================================================================
../../bases/source-acceptance-test/source_acceptance_test/config.py:257: 30 warnings
airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py: 1 warning
/Users/phil/Repos/phil/airbyte/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/config.py:257: DeprecationWarning: The 'warn' function is deprecated, use 'warning' instead
logging.warn("The acceptance-test-config.yml file is in a legacy format. Please migrate to the latest format.")

-- Docs: https://docs.pytest.org/en/stable/warnings.html

Results (26.17s):
31 passed

@octavia-squidington-iv octavia-squidington-iv added community connectors/source/bigcommerce area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Dec 15, 2022
@PhilipCorr PhilipCorr changed the title Phil/add brands and categories to bigcommerce 🎉 Source bigcommerce: add brands and categories streams Dec 15, 2022
@PhilipCorr PhilipCorr marked this pull request as ready for review December 15, 2022 14:48
@sajarin sajarin added the bounty-L Maintainer program: claimable large bounty PR label Dec 19, 2022
@marcosmarxm
Copy link
Member

Hello 👋:skin-tone-2: and thank you for your contribution!

Airbyte has instituted a code freeze between 19 and 30 December, to make sure there are no disruptions during the holidays.
Because of this, reviewing and merging your contribution may take longer than usual.
We apologize for the delay, but we want everyone to have a quiet and happy holiday.

If you have any questions or need further clarification, please don't hesitate to ping via Slack.

@haithem-souala haithem-souala self-assigned this Dec 20, 2022
@haithem-souala
Copy link
Contributor

Hey @PhilipCorr, thank you for your contribution.
Ill review your PR very soon.

@haithem-souala
Copy link
Contributor

haithem-souala commented Dec 21, 2022

/test connector=connectors/source-bigcommerce

🕑 connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3750893697
❌ connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3750893697
🐛 https://gradle.com/s/nsmifzmq456ma

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:94: The previous and actual specifications are identical.
======= 1 failed, 29 passed, 1 skipped, 31 warnings in 63.98s (0:01:03) ========

@haithem-souala haithem-souala self-requested a review December 21, 2022 17:02
@haithem-souala
Copy link
Contributor

Your PR looks good, i will publish the connector soon.

@haithem-souala
Copy link
Contributor

haithem-souala commented Dec 22, 2022

/publish connector=connectors/source-bigcommerce

🕑 Publishing the following connectors:
connectors/source-bigcommerce
https://github.com/airbytehq/airbyte/actions/runs/3757394824


Connector Did it publish? Were definitions generated?
connectors/source-bigcommerce

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@haithem-souala
Copy link
Contributor

haithem-souala commented Dec 22, 2022

/publish connector=connectors/source-bigcommerce run-tests=false

🕑 Publishing the following connectors:
connectors/source-bigcommerce
https://github.com/airbytehq/airbyte/actions/runs/3757843131


Connector Did it publish? Were definitions generated?
connectors/source-bigcommerce

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@haithem-souala
Copy link
Contributor

@sajarin this PR is ready to merge.
Thank you @PhilipCorr again, for your contribution!

@PhilipCorr
Copy link
Contributor Author

Hi @haithem-souala and @sajarin, are we happy to merge this PR? Thanks!

@haithem-souala haithem-souala enabled auto-merge (squash) January 5, 2023 13:08
@PhilipCorr
Copy link
Contributor Author

Hi @haithem-souala and @sajarin it would be great to get this merged soon if possible. It's starting to block some work that I have ongoing. Thanks!

@haithem-souala
Copy link
Contributor

Hi @haithem-souala and @sajarin it would be great to get this merged soon if possible. It's starting to block some work that I have ongoing. Thanks!

Hi @PhilipCorr, I am reaching out to some members of the Airbyte team to request the merging of this PR. As you may be aware, the merge process was on hold from December 19th to December 30th. I appreciate your patience.

@PhilipCorr
Copy link
Contributor Author

That's great, thanks for the help @haithem-souala!

@sajarin sajarin merged commit ceadf31 into airbytehq:master Jan 12, 2023
@sajarin
Copy link
Contributor

sajarin commented Jan 12, 2023

Thanks @PhilipCorr for the PR and thanks @haithem-souala for the review :)

jbfbell pushed a commit that referenced this pull request Jan 13, 2023
* Add product info to the orders stream

* Update docs

* Refactor order_products into its own stream

* formatting

* Update docs and docker version

* remove duplicate field

* Add brands and categories streams to bigcommerce

* remove duplicate state

* remove duplicate source

* update docs

* update docs

* bump docker version

* update markdown

* lint

* auto-bump connector version

Co-authored-by: Sajarin <sajarindider@gmail.com>
Co-authored-by: Haithem SOUALA <haithem.souala@woopit.fr>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation bounty bounty-L Maintainer program: claimable large bounty PR community connectors/source/bigcommerce
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

6 participants