mozilla / active-data-recipes

A repository of various activedata queries and recipes
Mozilla Public License 2.0
8 stars 24 forks source link

Traceback when running `adr try_usage` (due to missing data in ActiveData) #15

Closed ahal closed 5 years ago

ahal commented 6 years ago

The following traceback occurs when running adr try_usage:

Traceback (most recent call last):
  File "/home/ahal/.virtualenvs/adr/bin/adr", line 11, in <module>
    load_entry_point('active-data-recipes', 'console_scripts', 'adr')()
  File "/home/ahal/dev/active-data-recipes/adr/main.py", line 62, in cli
    print(run_recipe(recipe, remainder, fmt=args.fmt))
  File "/home/ahal/dev/active-data-recipes/adr/main.py", line 25, in run_recipe
    output = mod.run(args)
  File "/home/ahal/dev/active-data-recipes/adr/recipes/try_usage.py", line 17, in run
    count['total'] = len(data['message'])
KeyError: 'message'

This is happening because the repo data is missing from ActiveData (running adr try_usage --from now-3month works).

Rather than a traceback, we should print a nice error message and abort when this happens.

mUtterberg commented 6 years ago

screen shot 2018-09-20 at 7 37 07 am

Quick question: In working on this issue, I was unsure if there was an existing example of other functions that abort without traceback. Getting try_usage.run to return an error message (string) seems simple enough, but cli.run_recipe seems to require an instance in order to return fmt(output).

screen shot 2018-09-20 at 7 45 25 am

I'll keep looking, but am wondering if anyone has some quick redirection on that.

ahal commented 6 years ago

Good observation!

I think it would be fine to wrap the mod.run call in a try/except, print the error and skip the formatting in that case. It would be nice to create a custom Exception class that represents "ActiveData didn't return any data".

p.s It's true that all (or most) recipes will have some sort of problem if ActiveData doesn't return any data. I filed an issue for this particular recipe because it's reproducible now, so easier to see what the problem is and whether or not we fixed it.

So eventually I'd like to solve this for all recipes in a general sense. E.g we could validate that there is actual data in the run_query function. If you feel up for it we can do that here, but otherwise I'm happy to leave it for a separate issue.

mUtterberg commented 6 years ago

Sounds good! I took another look and noticed I'd hastily mixed up fmt and output, but yes there should at least be a check for empty queries. I ended up getting slammed today, but will address it tomorrow if no one beats me to it. (Which they're more than welcome to do.)

I didn't even think to check query.py for validation! Doesn't seem like that'd be too complex of an add-on. I'll at least look into it.