Adding Python DFA Reporting API Samples

Adding Python DFA Reporting API Samples

[Jonathon Imperiosi, 2013-11-13 14:41:38]

I've added DFA Reporting API code samples to the Python client library, please
review.

Associated issue for this CL:
https://code.google.com/p/google-api-python-client/issues/detail?id=313

[Joseph DiLallo, 2013-11-13 14:59:55]

Some comments. I commented on having two download reports examples (one using
files, one using reports.files). I'm not sure how useful I think that is;
they're basically identical in function. I think listing all reports via
reports.files and files is really a different use case (one is scoped by the
report) and deserves to exist as separate examples. I'm not so sure about
"get()"ing a single report using both methods; that too is basically identical.

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/create_re...
File samples/dfareporting/create_report.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/create_re...
samples/dfareporting/create_report.py:48: 'name': 'ENTER_NEW_REPORT_NAME',
I'd rather this report had a name like "Example Standard Report" or "New Report"
rather than "ENTER_NEW_REPORT_NAME". If you really want the user to pass it in,
make it a command line flag.

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/download_...
File samples/dfareporting/download_report_file.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/download_...
samples/dfareporting/download_report_file.py:17: """This example illustrates how
to download a file for a report.
I don't think we need an example for both ways to download a file. Either one
would be fine.

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_d...
File samples/dfareporting/get_all_dimension_values.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_d...
samples/dfareporting/get_all_dimension_values.py:17: """This example illustrates
how to get all dimension values for a dimension.
This doesn't get all dimension values, it gets the first page of dimension
values. Users would probably have to page, realistically.

I didn't page in the Java examples because I was using the first result for more
logic and the last page is empty. Since all of the examples are stand-alone
programs in Python, it may make sense for you to page through all of the
results.

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_f...
File samples/dfareporting/get_all_files.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_f...
samples/dfareporting/get_all_files.py:17: """This example illustrates how to get
a list of all the files for a profile.
You need to page to get all files

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_r...
File samples/dfareporting/get_all_report_files.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_r...
samples/dfareporting/get_all_report_files.py:17: """This example illustrates how
to get a list of all the files for a report.
You need to page to get all files

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_r...
File samples/dfareporting/get_all_reports.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_r...
samples/dfareporting/get_all_reports.py:17: """This example illustrates how to
get a list of all reports.
Same comment; you're fetching just the first page of results, not all reports. I
would page here. The other language examples do also

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_u...
File samples/dfareporting/get_all_userprofiles.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_all_u...
samples/dfareporting/get_all_userprofiles.py:17: """This example illustrates how
to get a list of all UserProfiles.
s/UserProfiles/user profiles

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_compa...
File samples/dfareporting/get_compatible_fields.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_compa...
samples/dfareporting/get_compatible_fields.py:55: # Create a report resource to
query
This won't work correctly; we don't use the ID to determine compatible fields,
we use the body of the report. (metricNames, fields, etc.) You're allowed to
pass in a report that you haven't saved yet. Plus I'm pretty sure this would
error out if you passed in certain types of reports, like a Floodlight report,
because some of the criteria fields are required.

Please use the entire report returned from the first request.

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_userp...
File samples/dfareporting/get_userprofile.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/get_userp...
samples/dfareporting/get_userprofile.py:17: """This example illustrates how to
get a UserProfile.
s/UserProfile/user profile

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/patch_rep...
File samples/dfareporting/patch_report.py (right):

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/patch_rep...
samples/dfareporting/patch_report.py:36: help='The ID of the report to patch')
I'd point out that this would only work for a standard report. Your docstring
and argument help field should say that. (Other report types don't use the
criteria field, they use floodlightCriteria, pathToConverionCriteria, etc.)

https://codereview.appspot.com/26070044/diff/1/samples/dfareporting/patch_rep...
samples/dfareporting/patch_report.py:49: # Create a report resource with the
fields to patch
Typically users don't patch so much in one go. (This is basically identical to
your create report example's report). I think it would be a more realistic use
case to just patch the criteria's dateRange, because that's the field users
mutate most often.

[Jonathon Imperiosi, 2013-11-13 21:24:21]

I've updated my samples to address all of Joseph's code review comments.
Unfortunately paging via the DFA Reporting API is a little wonky, so that code
isn't as clean as I'd like it to be.

[Joseph DiLallo, 2013-11-14 15:27:29]

1 more comment

https://codereview.appspot.com/26070044/diff/40001/samples/dfareporting/get_a...
File samples/dfareporting/get_all_dimension_values.py (right):

https://codereview.appspot.com/26070044/diff/40001/samples/dfareporting/get_a...
samples/dfareporting/get_all_dimension_values.py:61: nextPageToken =
response['nextPageToken']
We may be able to better future-proof ourselves by changing this to
  nextPageToken = response.get('nextPageToken')

This way if the DFA Reporting API is modified to stop returning the
nextPageToken on the last page, the example still works.

The same goes for all examples where you page.

[Jonathon Imperiosi, 2013-11-14 17:41:23]

Modified my paging examples, as suggested. Thanks for the tip!

[Joseph DiLallo, 2013-11-14 17:54:51]

LGTM

I suspect the Python API Client reviewer may ask you to revert that .hgignore,
but I'll leave that to him. Everything LGTM from a DFA Reporting standpoint.

[jcgregorio_google, 2013-11-14 19:06:35]

On 2013/11/14 17:54:51, Joseph DiLallo wrote:
> LGTM
> 
> I suspect the Python API Client reviewer may ask you to revert that .hgignore,
> but I'll leave that to him. Everything LGTM from a DFA Reporting standpoint.

LGTM

[Jonathon Imperiosi, 2013-11-15 21:24:58]

Just wanted to check and see if there's anything further I need to do to get
these examples committed. I don't believe I have the necessary permissions to
push the changes myself. Thanks!

[jcgregorio_google, 2013-11-18 16:18:03]

On 2013/11/15 21:24:58, Jonathon Imperiosi wrote:
> Just wanted to check and see if there's anything further I need to do to get
> these examples committed. I don't believe I have the necessary permissions to
> push the changes myself. Thanks!

Committed in
https://code.google.com/p/google-api-python-client/source/detail?r=6144ee7f09...