Skip to content
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

[WIP][20.09] Restore exception handling for item accessibility problems. #10459

Conversation

jmchilton
Copy link
Member

Shouldn't this fix #10309? This way the exception falls through the object collecting exceptions and reporting them back instead of short circuiting the workflow scheduling stuff.

This was added in f5c3478#diff-17974c8048767aa536063088ba97dbc9c4475e7589002c3b2335f9e8490fc920R1600 but it seems unrelated to the rest of that commit?

Shouldn't this fix galaxyproject#10309? This way the exception falls through the object collecting exceptions and reporting them back instead of short circuiting the workflow scheduling stuff.
@mvdbeek
Copy link
Member

mvdbeek commented Oct 20, 2020

I think that only changes the logged traceback, but doesn't fix #10309.
I do think we should be raising MessageExceptions, so that tool submissions show the proper exception in the UI.

Maybe we can catch MessageExceptions somewhere in the workflow scheduling stack and set some attribute on the invocation object that we can display in the UI ?

@jmchilton
Copy link
Member Author

Maybe we can catch MessageExceptions somewhere in the workflow scheduling stack and set some attribute on the invocation object that we can display in the UI ?

I guess I meant it fixes this part:

The job polling should also stop at this point, so users don't expect that things might still occur.

You're absolutely right about catching the message and passing it back to the user though. That should be straight-forward and a huge win.

@mvdbeek
Copy link
Member

mvdbeek commented Oct 20, 2020

The job polling should also stop at this point, so users don't expect that things might still occur.

Ah, that's true, but I think this prevents passing MessageExceptions back to the user when submitting a tool job (I think).
I should have written a bit more in the commit message.

@jmchilton
Copy link
Member Author

Yeah - that makes sense. I wonder if there is a way to propagate the errors better in the tool submission case.

@jmchilton jmchilton changed the title [20.09] Restore exception handling for item accessibility problems. [WIP][20.09] Restore exception handling for item accessibility problems. Oct 20, 2020
@jmchilton
Copy link
Member Author

Ah, that's true, but I think this prevents passing MessageExceptions back to the user when submitting a tool job (I think).
I should have written a bit more in the commit message.

I spent some time thinking about throwing the exception - but that seems wrong. We're dumping the errors out https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/webapps/galaxy/api/tools.py#L529 -- shouldn't the client just handle them better? Maybe we can make sure the errors are more structured as well.

@jmchilton jmchilton closed this Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants