singer-io / tap-pipedrive

A Singer.io tap for extracting data from the Pipedrive API
GNU Affero General Public License v3.0
13 stars 34 forks source link

Error for users which never logged in #31

Closed martint17r closed 5 years ago

martint17r commented 6 years ago

Hi,

I have a user which is deactivated and never logged in - this results in the following error:

2018-08-01 09:15:50,686Z target - INFO Sending batch with 40 messages for table users to https://api.stitchdata.com/v2/import/batch
2018-08-01 09:15:50,752Z target - CRITICAL Error persisting data for table "users": Record 0 did not conform to schema:
2018-08-01 09:15:50,752Z target - CRITICAL #: required key [last_login] not found
2018-08-01 09:15:55,784Z   main - WARNING Thread tap did not finish within timeout
2018-08-01 09:15:55,784Z   main - ERROR Target exited abnormally with status 1. Terminating tap.
2018-08-01 09:15:55,787Z   main - INFO Exit status is: Tap failed with code -15. Target failed with code 1 and error message: "Error persisting data for table "users": Record 0 did not conform to schema:
#: required key [last_login] not found".

To mitigate the incident I simply logged in, so there is no immediate need for action, more just a heads up.

martint17r commented 6 years ago

I got the same error, even though every user has a last_login JSON property set in the Pipedrive API response. Maybe there is another problem. I need to investigate further.

briansloane commented 6 years ago

@martint17r Thanks for the update. Let us know what you uncover with your investigation.

fitch commented 6 years ago

I'm getting the same error. Did any of you find anything regarding this?

timvisher commented 5 years ago

I believe the fix here would be to remove the required entry entirely from all schemas. It's not a json schema feature that we tend to use anywhere else and breaks the sync unnecessarily, especially because in this case there really is no last_login. last_login is already possibly null.

PR welcome!

bitgo-allenwu commented 5 years ago

hey Tim, I made a pull request to remove all the required entries: #45

KAllan357 commented 5 years ago

Released this in v0.2.3. Please close this issue if you verify things are working as expected.

bitgo-allenwu commented 5 years ago

It looks like everything's working now (for us, at least), though I don't think I can close the issue.

timvisher commented 5 years ago

@bitgo-allenwu FWIW you can always put (Closes|Fixes) #N somewhere either in the PR or in any of the commits and GitHub will automatically close the issue when the commits get merged.

bitgo-allenwu commented 5 years ago

Thanks! I'll keep that in mind for the future.