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

Cannot download CSV/Media file using DfareportingService#get_file #290

Closed
rafaelsales opened this issue Sep 30, 2015 · 16 comments
Closed

Cannot download CSV/Media file using DfareportingService#get_file #290

rafaelsales opened this issue Sep 30, 2015 · 16 comments
Assignees
Labels
🚨 This issue needs some love. triage me I really want to be triaged.
Milestone

Comments

@rafaelsales
Copy link

Issues: Option download_dest in DfareportingService#get_file does not work.

Testing file:

require 'google/apis/dfareporting_v2_1'
require 'hurley'

def auth
  @auth ||= begin
    scope = 'https://www.googleapis.com/auth/dfareporting'
    DoubleClick::Client.new(scope).authorization # DoubleClick::Client is my helper class for auth
  end
end

def service
  @service ||= Google::Apis::DfareportingV2_1::DfareportingService.new.tap do |service|
    service.authorization = auth
  end
end

def get_random_report_file(profile_id:, report_id:)
  service.list_report_files(profile_id, report_id).items.first do |file|
    file.status == 'REPORT_AVAILABLE'
  end
end

def download_ackward(file_metadata)
  file_get = Hurley.get(file_metadata.urls.api_url) do |request|
    request.header['Authorization'] = "Bearer #{auth.access_token}"
  end
  file_get.body
end

def download_beatiful(file_metadata)
  file_dest = '/tmp/report_beautiful.csv'
  service.get_file(file_metadata.report_id, file_metadata.id, download_dest: file_dest)
  File.new(file_dest).read
end

file_metadata = get_random_report_file(profile_id: 123, report_id: 456)

puts "--- With Ackward download method:"
puts download_ackward(file_metadata).lines[0..9]

puts "\n"
puts "--- With Beatiful download method:"
puts download_beatiful(file_metadata).lines[0..9]

Outcome:

--- With Ackward download method:
my-report

Date/Time Generated,9/29/15 4:48 AM
Report Time Zone,America/Los_Angeles
SubAccount Name,MyAccountName
Account ID,12345
Date Range,9/28/15 - 9/28/15

Report Fields
Campaign,Site (DCM),Date,Campaign End Date,Campaign Start Date,Campaign ID,Site ID (DCM),State/Region,Country,Placement,Placement ID,Site (Site Directory),Site ID (Site Directory),Advertiser,Advertiser ID,Impressions,Clicks,Click Rate,Cost Per Click,Effective CPM

--- With Beatiful download method:
Writing chunk (632 bytes)
Success - nil

<HTML>
<HEAD>
<TITLE>Temporary Redirect</TITLE>
</HEAD>
<BODY BGCOLOR="#FFFFFF" TEXT="#000000">
<H1>Temporary Redirect</H1>
The document has moved <A HREF="https://storage.googleapis.com/dfa_-my-report....csv?GoogleAccessId=abc@developer.gserviceaccount.com&amp;Expires=123&amp;Signature=xyz">here</A>.
</BODY>
</HTML>

Expectation: The get_file(..., download_dest: ...) should have downloaded the file correctly, instead if returns that "Temporary Redirect" page.

@sqrrrl
Copy link
Contributor

sqrrrl commented Sep 30, 2015

Will take a look at. Would be helpful if you can capture the raw HTTP response from the download request (e.g. using curl or whatever) since I don't have a DFA account I can use to test. Just need to verify that they're sending a proper redirect.

FYI, you can simplify the workaround a little - there's a generic http() method on the service that will
add the auth header and also supports writing directly to files:

service.http(:get, file_metadata.urls.api_url, download_dest: '/tmp/report.csv')

@sqrrrl sqrrrl added this to the 0.9 milestone Sep 30, 2015
@sqrrrl
Copy link
Contributor

sqrrrl commented Oct 2, 2015

Don't need the HTTP response. Was able to reproduce and am looking at a fix. Might be a little while though since the preferred fix involves some fixes to Hurley (http library)

@rafaelsales
Copy link
Author

Thank you!

@bruzos
Copy link

bruzos commented Oct 5, 2015

@rafaelsales did you use google services accounts to connect to the DCM webservice ?.

I need to work with that webservice but I have problems about permisions when my script connects with that Webservice using google service account.

Are you using that flow connection ?

Thank you

@bruzos
Copy link

bruzos commented Oct 8, 2015

I can confirm google account service doesn´t works with DCM if the google Apps Domain is not setup.

@rafaelsales
Copy link
Author

I couldn't access DFA reporting service with service account. Had to use client id, client secret and manually create a first refresh token.

@rafaelsales
Copy link
Author

btw, I tried with my company google apps

@bruzos
Copy link

bruzos commented Oct 9, 2015

Rafaelsales thank you for your reply.

I could make it work using installed flow. there is no needed to use google apps with this authentication flow, this way is recommended by google.

@rafaelsales
Copy link
Author

@bruzos when you say "it work" you mean the get_file method or just the auth? Cause in theory web and installed flow are not really different from each other: https://github.com/google/google-api-ruby-client/blob/669aaf01a401628c05c9081789ba4204919c6ac3/lib/google/api_client/client_secrets.rb#L29-L47
It's just that installed flow is little bit more convenient for mobile apps.

@bruzos
Copy link

bruzos commented Oct 13, 2015

Hi @rafaelsales I have tried your code with the same behavior you wrote. I think the problem is in the Hurley client it doesn't handle redirections.
Your are right, but google recommend as alternative to google service account use this flow, but if you manage by your self the access token life, I think it is ok

@rafaelsales
Copy link
Author

@bruzos I'm pretty sure Hurley actually does follow redirects: https://github.com/lostisland/hurley/blob/9be84b325c9a396a64268cd7bbca94080d9485ee/test/client_test.rb#L319

See the method download_ackward in the PR description - I use Hurley to download the CSV and it works.
In my opinion it's the google-api-ruby-client that is using the wrong URL to download the CSV, because I could make that work using file_metadata.urls.api_url.

@sqrrrl
Copy link
Contributor

sqrrrl commented Oct 13, 2015

It's a combination of api client + hurley bugs.

The issues with the client is it isn't expecting a redirect for downloads. APIs aren't supposed to redirect here, but a few do and the client should handle it. The problem is that Hurley is broken when it comes to streaming responses and redirects. There is a mechanism to filter streaming responses by status code, but it doesn't work. There's also a 2nd issue that it loses track of the fact that the caller wants to stream when a redirect happens. There are pending PRs (lostisland/hurley#28) and lostisland/hurley#34)) for both issues that I hope will be in the next release of Hurley. Once those are fixed I'll update the client to handle the redirect properly.

@bruzos
Copy link

bruzos commented Oct 14, 2015

@sqrrrl and @rafaelsales thank you very much. I will patiently wait for it :)

@rafaelsales
Copy link
Author

@sqrrrl thanks a lot for the explanation!

@bruzos
Copy link

bruzos commented Dec 3, 2015

Hi @sqrrrl it seem the pending PRs for Hurley client we was waiting for have been merged to master.

sqrrrl added a commit that referenced this issue Dec 16, 2015
… on 20x response. Includes temporary patch to Hurley until 0.3 released
@sqrrrl
Copy link
Contributor

sqrrrl commented Dec 16, 2015

They have been, but still not released so I can do a proper dependency on them. That said, I did hack in a fix for the next release which I'll push shortly.

@sqrrrl sqrrrl closed this as completed Dec 16, 2015
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚨 This issue needs some love. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

4 participants