Skip to content
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

Tdl 13587 add inventory item data #118

Merged
merged 15 commits into from
Oct 4, 2021

Conversation

dbshah1212
Copy link
Contributor

Description of change

Added the support of Inventory Item

QA steps

  • [Discover mode, Sync Mode] automated tests passing
  • [Check Discover mode, Sync mode and collect data for Inventory Item] manual qa steps passing (list below)

Risks

Rollback steps

  • revert this branch

@yogeshbhate
Copy link

yogeshbhate commented Sep 17, 2021

when would the PR's related to inventory items be merged ? is there any ETA ?

Copy link
Contributor

@KrisPersonal KrisPersonal left a comment

Choose a reason for hiding this comment

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

Overall looks good, but small change suggested

with self.subTest(store="store_1"):
conn_id = self.create_connection(original_credentials=True)
self.pagination_test(conn_id, self.store_1_streams)
self.pagination_test(conn_id, self.store_1_streams-excepted_streams)
Copy link
Contributor

Choose a reason for hiding this comment

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

Have spaces between store_1_streams and - and expected_streams

Suggested change
self.pagination_test(conn_id, self.store_1_streams-excepted_streams)
self.pagination_test(conn_id, self.store_1_streams - excepted_streams)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added


with self.subTest(store="store_2"):
conn_id = self.create_connection(original_properties=False, original_credentials=False)
self.pagination_test(conn_id, self.store_2_streams)
self.pagination_test(conn_id, self.store_2_streams-excepted_streams)
Copy link
Contributor

Choose a reason for hiding this comment

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

Include space as suggested above in line 26

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@luandy64 luandy64 mentioned this pull request Oct 1, 2021
2 tasks
@dbshah1212 dbshah1212 merged commit d1c12a9 into master Oct 4, 2021
tmck-code pushed a commit to lexerdev/tap-shopify that referenced this pull request Feb 3, 2022
* TDL-13587: Added inventory item data stream

* TDL-13587: Updated pagination test

* TDL-13587: Make pylint happy

* TDL-13587: Mkae pylint happy

* TDL-13587: Updated the logic of inventory item data collection

* TDL-13587: Make pylint happy

* TDL-13587: Rmoved white space

* TDL-13587: Added inventory_items stream in integration test

* TDL-13587: Updated pagination tests for inventory_item

* TDL-13587: Added unittests for inventory item data

* TDL-13587: Removed typo error

* TDL-13587: Updated readme

Co-authored-by: dbshah1212 <[email protected]>
Co-authored-by: savan-chovatiya <[email protected]>
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