-
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: Migrate from REST
> GraphQL BULK Operations
where possible, fixed STATE
collisions for sub-streams
#32345
🐛 Source Shopify: Migrate from REST
> GraphQL BULK Operations
where possible, fixed STATE
collisions for sub-streams
#32345
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
airbyte-integrations/connectors/source-shopify/source_shopify/utils.py
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.
lgtm
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.
the change makes sense to me. Can you add a test to confirm the fix (fine to do as a follow PR since we want to unblock the users)
The pre-release version of the
cc @girarda |
…fix-substream-state-filterring
…fix-substream-state-filterring
airbyte-integrations/connectors/source-shopify/source_shopify/utils.py
Outdated
Show resolved
Hide resolved
…ttps://github.com/airbytehq/airbyte into baz/source-shopify/fix-substream-state-filterring
…fix-substream-state-filterring
The new
Pre-release Notes:
|
…fix-substream-state-filterring
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.
we tested this with ~20 customers. let's 🚢
I'll merge this on Monday, February 26th, at 8:00 AM (PST) |
…re possible, fixed `STATE` collisions for `sub-streams` (airbytehq#32345)
…re possible, fixed `STATE` collisions for `sub-streams` (airbytehq#32345)
What
Resolving:
On-calls:
Regular issues:
How
Sub-streams
to avoid STATE regression while reading the streamfilter_records_newer_than_state
method to use thecached_substream_state
captures when the sync startsconfig
Exception to use theAibyteTracebackException
for config errorsGraphQL BULK Operations
to speed up the streams like:metafield_collections
metafield_customers
metafield_draft_orders
metafield_locations
metafield_orders
metafield_product_images
metafield_product_variants
collections
discount_codes
fulfillment_orders
inventory_items
inventory_levels
customer_address
transactions_graphql
(duplicatedtransactions
stream with reduced schema)stream
implementations fromsource.py
>streams.streams.py
base-class
implementations fromsource.py
>streams.base_streams.py
input configuration
optional fieldGraphQL BULK Date Range in Days
to manage BULK slices date range on demand.parent
and not fetch the data again usingrequests
(another performance boost):IncrementalShopifyNestedSubstream
and inherit from it:Product Images
Product Variants
Order Refunds
Fulfillments
concurrency checks
to hold on to thejob creating
ifanother job
is already running.unit_tests
to follow the current updatesunit_tests
🔴 User Impact 🔴
This PR introduces the breaking change for the :
Fulfillments
stream, by changing thecursor
fromid
toupdated_at
- the stream requiresreset
Product Images
stream, by changing thecursor
fromid
toupdated_at
- the stream requiresreset
Product Variants
stream, by changing thecursor
fromid
toupdated_at
- the stream requiresreset
Order Refunds
stream, by changing theschema.refund_line_items.line_item.properties
to array ofstrings
, instead ofobject
with properties.Collections
stream, a new scope is required to be added for all connections that useAPI_PASSWORD
auth option:read_publications
read_publications
and users should justre-auth
to pick up the new scope.Other changes are not considered as the
breaking changes
, butoptimization and performance improvements.
P.S.: The community post about migrating
OrderRisks
>GQL
: https://community.shopify.com/c/graphql-basics-and/get-the-orderrisks-full-data-using-graphql-not-rest-api/m-p/2353776/thread-id/12174The Breaking Changes doc, lives here: https://docs.google.com/document/d/1riyv36EEkkP-T1LK2CHeKyfR_uKCjI5sm1sb4CqqYu0/edit
The Additional QA Tests Results lives here: https://docs.google.com/document/d/1aZbeyw_DCwnsoOlV-5KL8yLauA8I_FQMV7uI0jhP4o0/edit#heading=h.onifjcf8uiqw