singer-io / tap-freshdesk

GNU Affero General Public License v3.0
15 stars 30 forks source link

Sync individual ticket endpoint to get tags. Ref #22 #23

Closed perplexes closed 5 years ago

perplexes commented 6 years ago

Makes an additional request for the ticket to pull tags. #22

cmerrick commented 6 years ago

Hi @perplexes, 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.

KAllan357 commented 6 years ago

@perplexes thanks for this contribution! Have you noticed any negative impact with making these extra requests and the freshdesk rate limits?

We'd like to move forward with this but need you to sign the CLA above.

perplexes commented 6 years ago

@KAllan357 unfortunately I haven't been able to test it in production or for long periods of time to test against rate limiting. I imagine it'll hit the rate limit more often (naturally).

We're circulating the CLA internally. It's the first one we've considered signing, so we're going over it carefully. Should have an update next week or so.

cmerrick commented 6 years ago

You did it @perplexes!

Thank you for signing the Singer Contribution License Agreement.

perplexes commented 6 years ago

@KAllan357 signed! :D

briansloane commented 6 years ago

@perplexes Have you tested this with any targets, such as the Stitch target, to determine if schema changes are necessary for the tap? Notably, if the tap now outputs records that don't conform to the defined schema, it could cause issues downstream in different targets.

KAllan357 commented 5 years ago

https://github.com/singer-io/tap-freshdesk/issues/22#issuecomment-452345348