singer-io / tap-kustomer

GNU Affero General Public License v3.0
2 stars 13 forks source link

Update config.json.example to include page_size_limit #13

Closed dRuEFFECT closed 3 years ago

dRuEFFECT commented 3 years ago

page_size_limit is supposed to be user configurable in order to account for hundreds of rows with the exact same updated timestamp. In Stitch, this is not available in the GUI and the integration runs with a fixed page_size_limit value of 100, causing pagination issues. comments in the sync.py script say this should be at least 300.

I don't assume this example file will directly affect the Stitch GUI, but the example file should include it at least.

Description of change

added page_size_limit

QA steps

(I don't know what to add here)

Risks

Rollback steps

cmerrick commented 3 years ago

Hi @dRuEFFECT, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

cmerrick commented 3 years ago

You did it @dRuEFFECT!

Thank you for signing the Singer Contribution License Agreement.

mascah commented 3 years ago

According to the Kustomer docs the pageSize limit is max 100

https://dev.kustomer.com/v1/customers/customer-search

How do you expect setting it to a value higher than 100 to behave?

ref: #12

asaf-erlich commented 3 years ago

For both PR https://github.com/singer-io/tap-kustomer/pull/13 and https://github.com/singer-io/tap-kustomer/pull/15 we're happy to merge it, but looking at the long discussion on the referenced issue https://github.com/singer-io/tap-kustomer/issues/12 I'm not convinced this will solve the root cause. I will work on merging them and then try to provide guidance on what I believe will better address the root cause (based on the information discussed).

asaf-erlich commented 3 years ago

I've pushed a local branch to trigger circle to run on this one.

asaf-erlich commented 3 years ago

Looks like circle was broken due to some known python version and dependency compatibility issues, so I'm going to create a PR to fix circle.

asaf-erlich commented 3 years ago

https://github.com/singer-io/tap-kustomer/pull/18

asaf-erlich commented 3 years ago

Now that the circle fix is merged I'm going to hit the button to update the branch.

asaf-erlich commented 3 years ago

Pushing the local branch again to trigger circle

asaf-erlich commented 3 years ago

The build is green, merging it. I'll proceed to do something similar for the other PR. After that I'll comment on the github issue of what I really suspect is the root cause.