Skip to content

Handle timeout gracefully for UWSGI connection#123

Merged
rhpvorderman merged 3 commits intomasterfrom
uwsgi_connection_aborted
Mar 18, 2019
Merged

Handle timeout gracefully for UWSGI connection#123
rhpvorderman merged 3 commits intomasterfrom
uwsgi_connection_aborted

Conversation

@pcm32
Copy link
Copy Markdown
Member

@pcm32 pcm32 commented Mar 15, 2019

I don't really know what I'm doing here, but following @mvdbeek suggestion :-).

@pcm32 pcm32 requested a review from mvdbeek March 15, 2019 15:11
@mvdbeek mvdbeek force-pushed the uwsgi_connection_aborted branch from d8a1111 to f7a04b9 Compare March 15, 2019 17:03
Copy link
Copy Markdown
Contributor

@rhpvorderman rhpvorderman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just one extra documenting comment (IMHO) and this can be merged.

if tool.get("model_class") == 'DataManagerTool':
repositories.append(get_repo_from_tool(tool))
repo = get_repo_from_tool(tool)
if repo:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea if a tool does not have a repo. Could you add a comment here with an example of why this is needed? For the people who read the code in the future and do not know all galaxy specifics by heart.

@mvdbeek
Copy link
Copy Markdown
Member

mvdbeek commented Mar 18, 2019

The download from NCBI doesn't seem to work currently, that may be an intermittent issue since it worked on Friday.

@rhpvorderman
Copy link
Copy Markdown
Contributor

Approved. The tests are running again if the issue was intermittent the PR should be ready to merge in a few minutes.
Thanks a lot @pcm32 and @mvdbeek for fixing this!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants