-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 Shopify: Add FulfillmentOrders
and Fulfillments
streams
#7107
🎉 Source Shopify: Add FulfillmentOrders
and Fulfillments
streams
#7107
Conversation
FulfillmentOrders
and Fulfillments
streams
Unittests
Integration tests
There is 1 error in integration tests that complain about some streams are empty. I think that is because the development store I used is not fully populated with data. |
airbyte-integrations/connectors/source-shopify/source_shopify/schemas/fulfillment_orders.json
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small changes! Overall LGTM!
Hello @marcosmarxm , thanks for the review comments. I have re-run the format according to the instruction. Please see if the PR is good to go. |
7fc7593
to
243ea76
Compare
integration test result after rebasing
|
@yuhuishi-convect will take a look in some time. Integration tests are passing ? |
/test connector=connectors/source-shopify
|
Thanks. Yes, the integration tests passed locally. Log
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with small corrections. Please check the comments.
airbyte-integrations/connectors/source-shopify/source_shopify/schemas/fulfillment_orders.json
Outdated
Show resolved
Hide resolved
}, | ||
"request_status": { | ||
"type": ["null", "string"], | ||
"enum": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about enum
block.
airbyte-integrations/connectors/source-shopify/source_shopify/schemas/fulfillment_orders.json
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-shopify/source_shopify/schemas/fulfillments.json
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-shopify/source_shopify/schemas/fulfillments.json
Outdated
Show resolved
Hide resolved
Thanks for the suggestions @bazarnov . Removed the enum types from schemas. integration tests
|
Hey can you update source-acceptance image |
Hi @harshithmullapudi , the integration tests all passed using the latest image log
|
LGTM, nicely done. |
Is there anything I can do to resolve the CI failure? I see one of the checks failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @yuhuishi-convect sorry the delay in merging your contribution
…irbytehq#7107) * Add FulfillmentOrder stream * Add fulfillments stream * Fix schema validation error * Bump docker version and update doc * Apply gradlew format * Fix source_definition * Fix doc after rebase * Fix format after rebase * Remove enum type in schemas * bump version * run seed config again Co-authored-by: Marcos Marx <[email protected]>
So this is why my backfill's gonna take an extra 43 hours... |
What
Describe what the change is solving
I am adding two new resources
FulfillmentOrders
andFulfillments
as new streams to Shopify connector.These two streams are critical if downstream apps rely on the demands at the warehousing location level (compared to the normal definition of order).
Closes #7026
Will land after #7063
It helps to add screenshots if it affects the frontend.
How
Describe the solution
FulfillmentOrders
stream is implemented as child stream ofOrders
- https://shopify.dev/api/admin-rest/2021-07/resources/fulfillmentorder#[get]/admin/api/2021-10/orders/{order_id}/fulfillment_orders.jsonFulfillments
stream is implemented as child stream ofOrders
- https://shopify.dev/api/admin-rest/2021-07/resources/fulfillment#[get]/admin/api/2021-10/orders/{order_id}/fulfillments.jsonRecommended reading order
source.py
schema/fulfillment_orders.py
schema/fulfillments.py
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described hereConnector Generator
-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