Export running data from Garmin API to CSV

Question

I'm interested in getting better. Current script does the job but I'm really unsure regarding

• Quality of overall structure & comments
• Error handling
• Possibility the add testing (what and how should I test?)

Here is the script:

# This script runs through your Garmin Connect activities (New->Old) and saves them as CSV
import json
import csv
import requests
from http.cookies import SimpleCookie

# To get started open garmin connect in Google Chrome, log in and grab the id of your latest activity as in https://connect.garmin.com/modern/activity/<START_ID>
START_ID = 2399797019 # Paste it here
END_ID = 0 # Id to stop at if you've run this script before, leave 0 otherwise

# Then, press F12, click on "Network", press F5, scroll to the very first element, it will have your <START_ID> as name. Click on it and copy the whole value of "Cookie" in Headers > Request Headers
RAW_DATA = 'SESSION=63497e03-bcbc-4...'

cookie = SimpleCookie()
cookie.load(RAW_DATA)
cookies = {}
for key, morsel in cookie.items():
    cookies[key] = morsel.value

# Crawl politely, leave a way for them to contact you
headers = {
    'User-Agent': 'Some hobbyist runner and pythonista trying to bulk extract CSVs, get in touch if you have an issue with this',
    'From': 'you@domain.com'
} 


def get_previous_activity_id(activity_id):
    url = 'https://connect.garmin.com/modern/proxy/activity-service/activity/{}/relative'.format(activity_id)
    r = requests.get(url, headers=headers, cookies=cookies)
    try:
        previous_activity_id =  r.json()["previousActivityId"]
        return(previous_activity_id)
    except KeyError:
        print('Issue getting the key ["previousActivityId"] for activity with id {}'.format(activity_id))
        return KeyError


def export_activity_laps(activity_id):
    url = 'https://connect.garmin.com/modern/proxy/download-service/export/csv/activity/{}'.format(activity_id)
    r = requests.get(url, headers=headers, cookies=cookies)
    try:
        with open('data/activities/{}.csv'.format(activity_id), 'w') as temp_file:
            temp_file.writelines(r.content.decode('utf-8'))
        return True
    except Exception as e:
        print('Issue writing the csv file for activity {}'.format(activity_id))
        return e


def export(activity_id):
    previous_id = get_previous_activity_id(activity_id)
    export_activity_laps(activity_id)
    if previous_id and previous_id != END_ID :
        export(previous_id)
    else:
        print("Finished exporting")


def main():
    export(START_ID)

if __name__ == '__main__':
    main()

Answer 1

Interesting script! Here are a few comments to make it better :)

Documentation

Documenting how to find the required values is a very nice touch, exactly the kind of things that helps a new user or maybe yourself in a few months. However, other parts of the documentation can be improved. Indeed, there is a Convention for Python docstrings. It is definitly worth a read. Among other things, your script documentation should be in """triple-quoted string""". Also, you should try to document your functions : purpose and when relevant parameters and return value (I'll come back to this).

Style

Not much to say about the style, your code looks good and seems to follow PEP 8. A few lines seem "too" long, maybe having the URLs as global constant would make things easier to read, maybe splitting the long literal strings on multiple lines could help as well.

Return value

What your functions return seems a bit unclear which probably makes them hard to use properly.

• get_previous_activity_id return an activity_id... or KeyError.
• export_activity_laps return True... or an exception.

Maybe, get_previous_activity_id should return None when it fails (or throw an exception depending on how expected this can be).

Maybe, export_activity_laps should not return anything at all and just throw in case of error.

Avoidable recursion

export relies on a recursion to iterate over the different activities. Python does not handle deep recursions very well because it does not perform Tail Recursion Optimisation. If needed, you can rewrite the function:

def export(activity_id):
    while True:
        previous_id = get_previous_activity_id(activity_id)
        export_activity_laps(activity_id)
        if not previous_id or previous_id = END_ID :
            break
    print("Finished exporting")

This version, just like yours, does not check the validity of the initial activity_id. This may be a desirable check. Then, you'd write something like:

def export(activity_id):
    while activity_id and activity_id != END_ID:
        export_activity_laps(activity_id)
        activity_id = get_previous_activity_id(activity_id)
    print("Finished exporting")

If you want to be a bit fancier and think you might use the logic retrieving the activity identifiers one after the other for a different usage, you could extract it in a generator:

def get_activity_identifiers(activity_id):
    while activity_id and activity_id != END_ID:
        yield activity_id
        activity_id = get_previous_activity_id(activity_id)

def export(activity_id):
    for activity_id in get_activity_identifiers(activity_id):
        export_activity_laps(activity_id)
    print("Finished exporting")

Parameters

It could make sense for export_activity_laps not to be responsible for chosing the file path. This could be provided as a parameter.

Disclaimer: I have run (pun intended) none of the code above.