singer-io / tap-fullstory

A Singer tap for extracting data from the FullStory API
GNU Affero General Public License v3.0
7 stars 9 forks source link

Upcoming change to FullStory Data Export API on 7/31/18 #2

Closed ian0608 closed 6 years ago

ian0608 commented 6 years ago

Hello from FullStory! It’s awesome to see FullStory represented as a Singer tap.

We wanted to make you aware that the base endpoint used by this tap (https://www.fullstory.com/api/v1/export/) will be shutting down on July 31st.

A while back, we transitioned to a new Data Export endpoint (https://export.fullstory.com/api/v1/export/). This is the endpoint that’s reflected in our Knowledge Base. So far we’ve been supporting both the new and old endpoints, but we’ll be sunsetting the old endpoint on 7/31. This tap will need to switch to https://export.fullstory.com/api/v1/export to continue functioning after that date. The API functionality remains the same- only the URL is different.

Thanks! Ian

dmosorast commented 6 years ago

@ian0608 Thanks for the heads up! I'm looking at this right now, and it seems that the new endpoint is no longer using Content-Type to specify if an export is GZip encoded (used here in the tap).

When the tap makes a request for https://export.fullstory.com/api/v1/export/get?id=..., the headers in the response are as follows:

ipdb> resp.headers
{'Date': 'Fri, 06 Jul 2018 14:01:10 GMT', 'Content-Type': 'application/json', 'Content-Disposition': 'attachment; filename=DataExport.json.gz', 'Content-Length': '26'}

The filename indicates it's a gzip file, but I'd rather not change the code to check for file extensions. Can the Content-Type be fixed? Where is the proper place to submit this feedback?

ian0608 commented 6 years ago

@dmosorast Thanks for bringing this up! We discussed internally, and we'll be changing our headers to match the below. This change should go live mid- next week.

If a client does not set the Accept-Encoding: gzip header on the request, we'll set the following headers on the response (this is the part being changed):

Content-Type: application/gzip
Content-Encoding: identity
Content-Disposition: attachment; filename=DataExport.json.gz

If a client does set Accept-Encoding: gzip, the response headers will look like this (this is already the existing behavior and won't change):

Content-Type: application/json
Content-Encoding: gzip
Content-Disposition: attachment; filename=DataExport.json

This should play nicely with the Singer tap code. As a side note... I thought Python requests would set Accept-Encoding: gzip automatically, and also automatically decode the response. Is there a reason this isn't being leveraged in the tap?

dmosorast commented 6 years ago

Thanks, @ian0608. That looks great! I was able to verify that the first set of headers will work just fine with the code as it stands.

As for requests, that's a good point. Curiosity got the best of me, so I dug around a bit, since I'm seeing gzipped content in the response object.

It appears that urllib3 will only decode the content portion of the response if decode_content is specified as True, which requests will pass in for reading the body as text. In this case, the reader in urllib3 will rely on the Content-Encoding header, which is covered under the second set you mentioned.

It appears that Requests isn't sending the Accept-Encoding header as expected, or the second case should work out automatically.

ian0608 commented 6 years ago

@dmosorast Ah, good catch. I definitely see the Accept-Encoding: gzip header being set when I use requests locally, so there must be some other weirdness going on. Regardless, I think we should be all set here once our code change goes live. I can close the issue at that time. Thanks again!

ian0608 commented 6 years ago

@dmosorast This change has been deployed- please let me know if you run into any issues.

ian0608 commented 6 years ago

On second thought, probably best to leave the issue open until the endpoints are changed in the tap code :)

dmosorast commented 6 years ago

Thanks, @ian0608! It looks like the tap code is working now. I'll make a PR to update the endpoint and reference/close this issue once it's merged.

dmosorast commented 6 years ago

Merged in #3