airbyte icon indicating copy to clipboard operation
airbyte copied to clipboard

Source bigcommerce: add channel and store stream

Open roisinbolt opened this issue 3 years ago • 1 comments

What

Add a source for Channels and Store to the source-bigcommerce integration

Closes https://github.com/airbytehq/airbyte/issues/16649

How

Channels: Channels has been added as a IncrementalBigcommerceStream.

I had to overwrite order_field bacause Channels API do not acept asc value

Store: Channels has been added as a BigcommerceStream.

I had to overwrite parse_response to ensure the store record is passed the format expected by downstream functions.

Recommended reading order

  1. configured_catalog.json
  2. channels.json
  3. store.json
  4. source.py

Pre-merge Checklist

Expand the relevant checklist and delete the others.

Updating a connector

Community member or Airbyter

  • [x] Grant edit access to maintainers (instructions)
  • [x] 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
  • [x] Documentation updated
    • [x] Connector's README.md
    • [x] Connector's bootstrap.md. See description and examples
    • [x] Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • [x] 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 currently no unit tests in the source-bigcommerce integration. Is it expected for me to add these tests along with this PR?

Integration

There are no integration tests present in the source-bigcommerce folder

Acceptance

The acceptance tests are all error out with a 404 Client Error locally

roisinbolt avatar Sep 13 '22 16:09 roisinbolt

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Sep 13 '22 16:09 CLAassistant

/test connector=connectors/source-bigcommerce

:clock2: connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3053934687 :x: connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3053934687 :bug: https://gradle.com/s/xrridbiwm5iii

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Please check...
FAILED test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
======================== 2 failed, 27 passed in 49.23s =========================

marcosmarxm avatar Sep 14 '22 15:09 marcosmarxm

@marcosmarxm Thanks for running the integration tests! I have just pushed a fix for test_core, I am still looking into the test_incremental failure. I am struggling to recreate this failure locally - it appears to be running TestIncremental for the full_refresh store stream.

I have updated store abnormal_state.json which may resolve this - is it possible to re-run the tests?

roisinbolt avatar Sep 14 '22 17:09 roisinbolt

/test connector=connectors/source-bigcommerce

:clock2: connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3061573422 :x: connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3061573422 :bug: https://gradle.com/s/phxdfkr5fb3is

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
======================== 1 failed, 28 passed in 53.69s =========================

marcosmarxm avatar Sep 15 '22 15:09 marcosmarxm

Hey @marcosmarxm thanks for re-running! I think I found the error - I had a state present for in abnormal_state.json and sample_state.json for the full_refresh store stream. So I have removed these two entries and that should have resolved the failure.

roisinbolt avatar Sep 15 '22 15:09 roisinbolt

Am I able to re-run the tests from my side?

roisinbolt avatar Sep 15 '22 16:09 roisinbolt

/test connector=connectors/source-bigcommerce

:clock2: connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3069133717 :x: connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3069133717 :bug: https://gradle.com/s/ac7jh2vl34k34

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
======================== 1 failed, 28 passed in 51.97s =========================

marcosmarxm avatar Sep 16 '22 15:09 marcosmarxm

@sajarin can you request a maintainer to review this contribution?

marcosmarxm avatar Sep 16 '22 16:09 marcosmarxm

Thank you for the PR @roisinbolt. This PR is part of Airbyte's Community Maintainer program and will be reviewed by a member of our community. A maintainer will be assigned shortly to help review and get this PR merged into our main branch. Thank you for being patient, we hope to get this merged soon!

sajarin avatar Sep 21 '22 14:09 sajarin

Hey @sajarin, just wanted to check in if there was any progress on this?

roisinbolt avatar Sep 26 '22 08:09 roisinbolt

@roisinbolt, sorry for the wait. I'll prioritize assigning someone to this PR!

sajarin avatar Sep 26 '22 16:09 sajarin

/test connector=connectors/source-bigcommerce

:clock2: connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3164818238 :white_check_mark: connectors/source-bigcommerce https://github.com/airbytehq/airbyte/actions/runs/3164818238 Python tests coverage:

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       152     26    83%   21-23, 29-31, 36-43, 48-61, 239, 250-258
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-67, 70-72, 75-77, 80-82, 85-87, 90-92, 95-113, 147-149
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1358    466    66%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_incremental.py:23: `future_state_path` not specified, skipping
======================== 28 passed, 1 skipped in 44.76s ========================

vincentkoc avatar Oct 01 '22 13:10 vincentkoc

/publish connector=connectors/source-bigcommerce

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

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

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

vincentkoc avatar Oct 01 '22 14:10 vincentkoc

@koconder Thanks for all your help! Is this ready to be merged?

roisinbolt avatar Oct 03 '22 08:10 roisinbolt

@roisinbolt yes please merge

vincentkoc avatar Oct 03 '22 10:10 vincentkoc

@koconder I don't have permission to merge this PR, if you have a minute would you be okay to do so?

roisinbolt avatar Oct 03 '22 10:10 roisinbolt

@sajarin can we please merge, some reason i am unable to merge

vincentkoc avatar Oct 03 '22 12:10 vincentkoc

Thanks for the PR @roisinbolt and thanks for the review @koconder!

sajarin avatar Oct 03 '22 14:10 sajarin