Repository: getredash/redash
Pull request: Google analytics fix fixes #2965
resolves #2965

Problem statement:
GA Data Source throws an error when no rows returned
### Issue Summary

Google Analytics Data Source throws `Error running query: 'rows'` when the query result is empty.
I have a pretty simple query with dimensions and filters, like:

```json
{
  "ids": "ga:177xxxxxx",
  "start_date": "2018-10-08",
  "end_date": "2018-10-12",
  "metrics": "ga:uniqueEvents",
  "dimensions": "ga:dimension1,ga:dimension3",
  "filters": "ga:dimension2==userrole;ga:eventCategory==eventcategory;ga:eventAction==enentaction;ga:dimension1!=demo"
}
```

Sometimes it returns empty result as there is no data. This results in error in redash.

### Steps to Reproduce

1. Create the Google Analytics Data Source
2. Make some query returning zero rows
3. Execute it in query editor

`Error running query: 'rows'` will be thrown. While this might be considered not a bug, I'd expect just an empty result with no errors.

### Technical details:

* Redash Version: 5.0.1
* Browser/OS: Chrome/macOS
* How did you install Redash: docker-compose

Hints from issue discussion:

Commit message: resolves #2965

Patch to review:
diff --git a/redash/query_runner/google_analytics.py b/redash/query_runner/google_analytics.py
index e8b70eb01f..19def81243 100644
--- a/redash/query_runner/google_analytics.py
+++ b/redash/query_runner/google_analytics.py
@@ -43,36 +43,37 @@ def parse_ga_response(response):
         })
 
     rows = []
-    for r in response['rows']:
-        d = {}
-        for c, value in enumerate(r):
-            column_name = response['columnHeaders'][c]['name']
-            column_type = filter(lambda col: col['name'] == column_name, columns)[0]['type']
-
-            # mcf results come a bit different than ga results:
-            if isinstance(value, dict):
-                if 'primitiveValue' in value:
-                    value = value['primitiveValue']
-                elif 'conversionPathValue' in value:
-                    steps = []
-                    for step in value['conversionPathValue']:
-                        steps.append('{}:{}'.format(step['interactionType'], step['nodeValue']))
-                    value = ', '.join(steps)
-                else:
-                    raise Exception("Results format not supported")
-
-            if column_type == TYPE_DATE:
-                value = datetime.strptime(value, '%Y%m%d')
-            elif column_type == TYPE_DATETIME:
-                if len(value) == 10:
-                    value = datetime.strptime(value, '%Y%m%d%H')
-                elif len(value) == 12:
-                    value = datetime.strptime(value, '%Y%m%d%H%M')
-                else:
-                    raise Exception("Unknown date/time format in results: '{}'".format(value))
-
-            d[column_name] = value
-        rows.append(d)
+    if response and 'rows' in response:
+        for r in response['rows']:
+            d = {}
+            for c, value in enumerate(r):
+                column_name = response['columnHeaders'][c]['name']
+                column_type = filter(lambda col: col['name'] == column_name, columns)[0]['type']
+
+                # mcf results come a bit different than ga results:
+                if isinstance(value, dict):
+                    if 'primitiveValue' in value:
+                        value = value['primitiveValue']
+                    elif 'conversionPathValue' in value:
+                        steps = []
+                        for step in value['conversionPathValue']:
+                            steps.append('{}:{}'.format(step['interactionType'], step['nodeValue']))
+                        value = ', '.join(steps)
+                    else:
+                        raise Exception("Results format not supported")
+
+                if column_type == TYPE_DATE:
+                    value = datetime.strptime(value, '%Y%m%d')
+                elif column_type == TYPE_DATETIME:
+                    if len(value) == 10:
+                        value = datetime.strptime(value, '%Y%m%d%H')
+                    elif len(value) == 12:
+                        value = datetime.strptime(value, '%Y%m%d%H%M')
+                    else:
+                        raise Exception("Unknown date/time format in results: '{}'".format(value))
+
+                d[column_name] = value
+            rows.append(d)
 
     return {'columns': columns, 'rows': rows}
 


Code review comments:
diff_hunk: @@ -43,36 +43,37 @@ def parse_ga_response(response):
         })
 
     rows = []
-    for r in response['rows']:
-        d = {}
-        for c, value in enumerate(r):
-            column_name = response['columnHeaders'][c]['name']
-            column_type = filter(lambda col: col['name'] == column_name, columns)[0]['type']
-
-            # mcf results come a bit different than ga results:
-            if isinstance(value, dict):
-                if 'primitiveValue' in value:
-                    value = value['primitiveValue']
-                elif 'conversionPathValue' in value:
-                    steps = []
-                    for step in value['conversionPathValue']:
-                        steps.append('{}:{}'.format(step['interactionType'], step['nodeValue']))
-                    value = ', '.join(steps)
-                else:
-                    raise Exception("Results format not supported")
-
-            if column_type == TYPE_DATE:
-                value = datetime.strptime(value, '%Y%m%d')
-            elif column_type == TYPE_DATETIME:
-                if len(value) == 10:
-                    value = datetime.strptime(value, '%Y%m%d%H')
-                elif len(value) == 12:
-                    value = datetime.strptime(value, '%Y%m%d%H%M')
-                else:
-                    raise Exception("Unknown date/time format in results: '{}'".format(value))
-
-            d[column_name] = value
-        rows.append(d)
+    if response and 'rows' in response:
+        for r in response['rows']:
path: redash/query_runner/google_analytics.py
text: @user1:
We can assume `response` exists because otherwise this function will fail when trying to load the columns. In this case, maybe we can simplify this with a `for r in response.get('rows', []):`, so we don't need the extra indentation? 

@author:
ok.
done!