🎉 Source Shopify: update the Shopify api version to 2024-04
#38610
Conversation
…ify/fix-streams-regressions-after-swith-to-2024-04
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…ify/fix-streams-regressions-after-swith-to-2024-04
octavia-squidington-iii
added
the
area/documentation
airbyte-integrations/connectors/source-shopify/integration_tests/expected_records.jsonl
Show resolved
Hide resolved
|
|
||
| return Query( | ||
| name="quantities", | ||
| arguments=[Argument(name="names", value=self.quantities_names_filter)], |
There was a problem hiding this comment.
Why do we need this argument? Can't we just get everything? Could we have dataloss if a new quantity.name is added?
There was a problem hiding this comment.
No, we can't. This is the part of the nested query building process, let's keep this simple.
There was a problem hiding this comment.
What do you mean by get just everything?
There was a problem hiding this comment.
My understanding by the name of it is that this is a filter meaning that we fetch only the values that are defined in self.quantities_names_filter. So effectively, this generate this part of the query:
quantities(names: <list represented by self.quantities_names_filter>) {
<fields>
}
Can we just query for everything by not passing this filter like this?
quantities {
<fields>
}
There was a problem hiding this comment.
Got your point, we fetch all the values available, there is no wildcard fetch for this, by non-specifying these types. We have to pass all of them as it is now.
There was a problem hiding this comment.
Here is some more info:
"errors": [
{
"message": "names has an invalid value. Possible values include: available, committed, damaged, incoming,
on_hand, quality_control, reserved, safety_stock.",
...
}
]
There is no opportunity to pass * or ALL, or something else.
The official doc is here: https://shopify.dev/docs/apps/fulfillment/inventory-management-apps#inventory-states
There was a problem hiding this comment.
I've added the reference link to the code, for quick tips about that, as well.
There was a problem hiding this comment.
The other possibility I see is an integration tests that query for a value we know does not exist and ensure the error message contains only the possible values we know of. Is this something you think is worth?
There was a problem hiding this comment.
We've decided for now that this isn't worth the hassle of creating a test that query for an error to validate the names. However, we will contact Shopify support to ask them if they can accept a wildcard on it and see how they react
There was a problem hiding this comment.
Created an Issue for the Shopify Comunity with the question and improvement suggestion: https://community.shopify.com/c/graphql-basics-and/graphql-inventorylevels-quantities-issue-with-names-state-filter/m-p/2587605#M13215
airbyte-integrations/connectors/source-shopify/source_shopify/shopify_graphql/bulk/query.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-shopify/source_shopify/shopify_graphql/bulk/query.py
Show resolved
Hide resolved
|
The Shopify community issue about the mandatory |
bazarnov
deleted the
baz/baz/source/shopify/fix-streams-regressions-after-swith-to-2024-04
branch
What
Resolving:
How
Inventory_levelsstream schema, by adding newquantitiesproperty, instead ofavailabile(it's deprecated for2024-04)Collectionsstream BULKqueryto guarantee havingproducts_countwith the new2024-04Products_graph_qlstream to use the2023-07API version to ensure it works as expected before we deprecate it.expected records, since we have added new fieldsunit tests/graphql_bulk/:test_query.pyandtest_job.pyto cover the changes.User Impact
Refresh Schema>Apply changes, with no reset.Can this PR be safely reverted and rolled back?