New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not all exceptions will have a .read() method. #155

Merged
merged 1 commit into from Apr 30, 2015

Conversation

Projects
None yet
3 participants
@peterjc
Copy link
Contributor

peterjc commented Apr 30, 2015

Intended to partly address GitHub issue #116 (but the exception handling is still nasty). Before this change,

$ planemo shed_upload --shed_target testtoolshed --message "v0.0.8a again (testing uploads via planemo with API key)" --tar ~/toolshed_archives/venn_list_v0.0.8a.tar.gz ~/repositories/pico_galaxy/tools/venn_list
Traceback (most recent call last):
  File "/mnt/galaxy/repositories/planemo/.venv/bin/planemo", line 9, in <module>
    load_entry_point('planemo==0.9.0.dev0', 'console_scripts', 'planemo')()
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/click-4.0-py2.6.egg/click/core.py", line 664, in __call__
    return self.main(*args, **kwargs)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/click-4.0-py2.6.egg/click/core.py", line 644, in main
    rv = self.invoke(ctx)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/click-4.0-py2.6.egg/click/core.py", line 991, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/click-4.0-py2.6.egg/click/core.py", line 837, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/click-4.0-py2.6.egg/click/core.py", line 464, in invoke
    return callback(*args, **kwargs)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/click-4.0-py2.6.egg/click/decorators.py", line 64, in new_func
    return ctx.invoke(f, obj, *args[1:], **kwargs)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/click-4.0-py2.6.egg/click/core.py", line 464, in invoke
    return callback(*args, **kwargs)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/planemo-0.9.0.dev0-py2.6.egg/planemo/commands/cmd_shed_upload.py", line 63, in cli
    exit_code = shed.for_each_repository(upload, path, **kwds)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/planemo-0.9.0.dev0-py2.6.egg/planemo/shed.py", line 422, in for_each_repository
    function(realized_repository)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/planemo-0.9.0.dev0-py2.6.egg/planemo/commands/cmd_shed_upload.py", line 61, in upload
    return __handle_upload(ctx, realized_repository, **kwds)
  File "/mnt/galaxy/repositories/planemo/.venv/lib/python2.6/site-packages/planemo-0.9.0.dev0-py2.6.egg/planemo/commands/cmd_shed_upload.py", line 99, in __handle_upload
    exception_content = e.read()
AttributeError: 'ConnectionError' object has no attribute 'read'

With this fix:

$ planemo shed_upload --shed_target testtoolshed --message "v0.0.8a again (testing uploads via planemo with API key)" --tar ~/toolshed_archives/venn_list_v0.0.8a.tar.gz ~/repositories/pico_galaxy/tools/venn_list
Could not update venn_list
Unexpected response from galaxy: 400: {"content_alert": "", "err_msg": "No changes to repository."}

Clearly still room for improvement - this looks like JSON so we ought to be able to pull out the error message as the older error handling code tries to do.

Not all exceptions will have a .read() method.
Intended to resolve GitHub issue #116 (but the exception handling is still nasty)
@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 30, 2015

@erasche This is only a short term solution until you can sort out #116 properly.

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Apr 30, 2015

Thanks for the fix @peterjc - I'll tweak it a little because it is causing the make lint to fail - but it is a solid improvement.

jmchilton added a commit that referenced this pull request Apr 30, 2015

Merge pull request #155 from peterjc/shed_upload_errors
Not all exceptions will have a .read() method.

@jmchilton jmchilton merged commit cd0b47a into galaxyproject:master Apr 30, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 30, 2015

The second .read() was bothering me too. Thanks.

@erasche

This comment has been minimized.

Copy link
Member

erasche commented Apr 30, 2015

I'll finish up with #116 sometime today.

tor. 30. apr. 2015 kl. 07.31 skrev John Chilton notifications@github.com:

Merged #155 #155.


Reply to this email directly or view it on GitHub
#155 (comment).

@peterjc peterjc deleted the peterjc:shed_upload_errors branch Apr 30, 2015

@erasche

This comment has been minimized.

Copy link
Member

erasche commented Apr 30, 2015

It ALL bothers me. It's ghastly but I was hitting all manner of different, awful bioblend/TS exceptions and so I hacked together something functional. Then I started bumping up against the --max-complexity limit and it just went downhill from there... I'm sure I can develop something more elegant.

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 30, 2015

Good luck - there seems to be a lot of very different exceptions to think about here :(

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Apr 30, 2015

Crap - didn't see you were going to work on this and I tried to fix it for travis. What do we think about - https://github.com/galaxyproject/planemo/compare/master...jmchilton:exception_handling?expand=1.

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 30, 2015

Assuming api_exception_to_message(...) would get used elsewhere, that seems like a sensible refactoring.

@erasche

This comment has been minimized.

Copy link
Member

erasche commented Apr 30, 2015

No worries @jmchilton, lgtm. :) Let's pretend I implemented it that nicely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment