Skip to content

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

Merged
jmchilton merged 1 commit intogalaxyproject:masterfrom
peterjc:shed_upload_errors
Apr 30, 2015
Merged

Not all exceptions will have a .read() method.#155
jmchilton merged 1 commit intogalaxyproject:masterfrom
peterjc:shed_upload_errors

Conversation

@peterjc
Copy link
Contributor

@peterjc 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.

@peterjc
Not all exceptions will have a .read() method.
cd049b4 
Intended to resolve GitHub issue galaxyproject#116 (but the exception handling is still nasty)
@peterjc
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
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
@jmchilton
Merge pull request #155 from peterjc/shed_upload_errors
cd0b47a 
Not all exceptions will have a .read() method.
@jmchilton jmchilton merged commit cd0b47a into galaxyproject:master Apr 30, 2015
@peterjc
Copy link
Contributor Author

peterjc commented Apr 30, 2015

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

@hexylena
Copy link
Member

hexylena 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 shed_upload_errors branch April 30, 2015 12:39
@hexylena
Copy link
Member

hexylena 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
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
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
Copy link
Contributor Author

peterjc commented Apr 30, 2015

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

@hexylena
Copy link
Member

hexylena 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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@peterjc @jmchilton @hexylena