Skip to content

Migrate to GraphQL Streams#193

Merged
vishalp-dev merged 61 commits into
masterfrom
implement-products-gql
Jan 24, 2025
Merged

Migrate to GraphQL Streams#193
vishalp-dev merged 61 commits into
masterfrom
implement-products-gql

Conversation

@vishalp-dev

@vishalp-dev vishalp-dev commented Jan 17, 2025

Copy link
Copy Markdown
Member

Description of change

This PR Migrates few streams to graphQl due to the upcoming sunset on 01/02/25

Integration tests will be updated in a different pr

Streams that are migrated:

  • Products
  • Inventory Items (prev. dependancy on products)
  • Metafields
  • Product Variants (new stream added)

What is changed in each stream

Products

- Primary Key int -> string
- Replication key updated_at -> updatedAt
- Removed Images and Image fields
- Variants now available as a new stream
- removed dependant streams (metafields and Inventory Items)
- other minor schema changes and new field additions

Inventory Items

- Primary Key int -> string
- Replication key updated_at -> updatedAt
- minor schema name and structure change, no new fields

Metafields

- Primary Key int -> string
- Replication key updated_at -> updatedAt
- minor schema name and structure change, no new fields
- Removed dependancy on parent streams (orders, inventory items, products, customers)
- All parents are fetched through graphql to maintain consistency between schema and primary keys

Product Variants

- New Stream addition (migrated from product stream)

Structural Changes

  • Added base class implementation for graphQL Streams
  • Added Queries for each graphQl stream

QA steps

  • automated tests passing
  • manual qa steps passing (list below)
  • ran discovery and checked schema
  • ran sync and checked records for transformation issues
  • ran with bookmarks to confirm functionality
  • checked pagination with lower page size limits
  • Checked Record Count for Discrepancies

Risks

Rollback steps

  • revert this branch

AI generated code

https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code

  • this PR has been written with the help of GitHub Copilot or another generative AI tool

@vishalp-dev vishalp-dev marked this pull request as draft January 17, 2025 06:51
@vishalp-dev vishalp-dev marked this pull request as ready for review January 21, 2025 08:32
yield obj

page_info = data.get("pageInfo")
cursor , has_next_page = page_info.get("endCursor"), page_info.get("hasNextPage")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cursor , has_next_page = page_info.get("endCursor"), page_info.get("hasNextPage")
cursor, has_next_page = page_info.get("endCursor"), page_info.get("hasNextPage")

@RushiT0122 RushiT0122 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see amount, Price fields are marked string in schema, should those be singer.decimals?

Comment on lines +42 to +47
resource_alias = {
"customers":"customer",
"products":"product",
"collections": "collection",
"orders": "order"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor fix, won't block the PR but get it fixed.

Suggested change
resource_alias = {
"customers":"customer",
"products":"product",
"collections": "collection",
"orders": "order"
}
resource_alias = {
"customers": "customer",
"products": "product",
"collections": "collection",
"orders": "order"
}

@RushiT0122 RushiT0122 self-requested a review January 23, 2025 15:15
@RushiT0122

Copy link
Copy Markdown
Contributor

Will wait for integration test PR to get merged for final approval.

vishalp-dev and others added 5 commits January 24, 2025 08:40
…194)

* Add unit test for the graphql scenarios

* add teardown

* remove metafields from the bookmarks test

* add new integ. test for metafields

* update foreign keys within meta

* fix the failing interrupt sync

* Include metafields shop in the bookmarks test

* Changes as per the suggestions

* update doc string

* Camel case the comments

* make metafield_dict a object variable

* remove trailing spaces
@vishalp-dev

Copy link
Copy Markdown
Member Author

I see amount, Price fields are marked string in schema, should those be singer.decimals?

I have checked the schema and updated the relevant fields to use singer.decimals

@bslayton

Copy link
Copy Markdown

@somethingmorerelevant Are there plans to update the other streams? Specifically events? I believe I found a bug related to events but it's irrelvant with the REST API being depreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants