search

mozilla / active-data-recipes

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

Replacing traceback with error message on empty query (try_usage) #26

Closed mUtterberg closed 6 years ago

mUtterberg commented 6 years ago

Addresses issue #15

I didn't think to run tox before making changes, so I'm not 100% sure the ERROR: InvocationError for command '/active-data-recipes/.tox/flake8/bin/flake8 adr' (exited with code 1) was introduced by my change. However, it does pass all other tests.

Changed try_usage.py and cli.py only. Could use refactor as time allows, but currently returns the following error message rather than the previous traceback:

Error: The queried data does not exist. (ActiveData didn\'t return any data.)

mUtterberg commented 6 years ago

Dangit! Ok clearly that flake8 error matters. Working on it.

ahal commented 6 years ago

No worries :)

You can test it out locally by running:

$ pip install tox
$ tox

If that passes, you can be pretty confident that the TravisCI will also pass. You could also just run flake8 directly to avoid the unittests if you want.

I will ask that you squash your commits though so it's easier for me to review them. A common way of doing this is with git rebase -i <base commit>. E.g:

$ git rebase -i master

(you'll need to use -f to push afterwards, but that's fine when you're pushing to a PR on your local fork)

Thanks for the PR!

mUtterberg commented 6 years ago

Totally makes sense! Will do.

mUtterberg commented 6 years ago

Squashed! Yeah, that looks way cleaner.

mUtterberg commented 6 years ago

Gotcha! I think squashing this most recent change made it so that I can't comment directly on your review, but I pushed that change too soon. One more revision should do it, hopefully. X-/

mUtterberg commented 6 years ago

Ok THIS was what I meant to push.

ahal commented 6 years ago

I fixed the conflicts, but the function you were modifying in cli.py (run_recipe) got moved over to recipe.py. Github's conflict resolution UI didn't allow me to modify that file, so the changes you made in there got lost. But I think that's ok as that was the part you needed to change anyway :). Also looks like my commit broke flake8, my apologies! Shouldn't be hard to sort out though.

mUtterberg commented 6 years ago

No worries! Flake8 errors are pretty much the easiest to resolve.

ahal commented 6 years ago

Thanks! Are you interested in working on anything else? If so feel free to comment in any issues that look interesting to you.