-
Notifications
You must be signed in to change notification settings - Fork 966
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
planemo tests timing out and ASGI error #13136
Comments
Can you please check if galaxy at the commit before that also exhibit the same issue? #13135 updated starlette and fastapi, and could be the cause for BrokenResourceError. |
Did a
|
For a5b1452 the changes in |
Can you check if #13137 fixes it for you ? I could see the test hang if the metadata is in error ... . If that is true we need to find a way to error out quickly. |
If no check or skip is given, self.check and self.skip need to be initialised to None. Otherwise validation with `value.missing_meta(check=self.check, skip=self.skip)` is called with `value.missing_meta(check=[""], skip=[""])`. This led to the stalling observed in galaxyproject#13136 Explanation: each data input parameter automatically gets a MetadataValidator. Since this validator was dysfunctional the job was scheduled anyway, but did not execute.
@davelopez do we still need to downgrade fastAPI ? I see a lot of |
Yeah, I also thought it was fixed but I guess it is still an issue tiangolo/fastapi#4041 so probably we should downgrade again. |
Is it a real issue of just polluting the logs? |
That's a good question, some people claim it is fixed and some are silencing it but I'm afraid I don't know if that's a good idea or what are the consequences. |
Yeah, all of these are kind of "logging issues". It happens when the client goes away and we write to a closed socket. You can see that the fix in https://github.com/encode/starlette/pull/1262/files is to pick up the right exception from the coroutine, I suppose that something similar should be done for the WSGI middleware we're using (which doesn't seem to be based on BaseHTTPMiddleware that got fixed). A good start might be to write a small client test |
It might not be the closed socket issue .... import time
import uvicorn
from fastapi import FastAPI
from fastapi.middleware.wsgi import WSGIMiddleware
def wsgi_application(env, start_response):
print('start')
time.sleep(0.3)
start_response("200 OK", {})
print('done')
return [b'bla']
app = FastAPI()
app.mount('/', WSGIMiddleware(wsgi_application))
if __name__ == "__main__":
uvicorn.run(app="timeoutapp:app", port=8001) And then |
Describe the bug
When running
planemo test
on a few (two) tools of the IUC repo I see the following error after Galaxy started and the tool test execution stalls reproducibly.Tool tests stall at a line similar to (only ids differ):
Galaxy Version and/or server at which you observed the bug
Galaxy Version: dev
Commit: 060ab1a
To Reproduce
Steps to reproduce the behavior:
Run either
planemo test --biocontainers --no_dependency_resolution --no_conda_auto_init --galaxy_python_version 3.7 --galaxy_source https://github.com/galaxyproject/galaxy --galaxy_branch dev tools/macs2/macs2_callpeak.xml
orplanemo test --biocontainers --no_dependency_resolution --no_conda_auto_init --galaxy_python_version 3.7 --galaxy_source https://github.com/galaxyproject/galaxy --galaxy_branch dev tools/pygenometracks/pyGenomeTracks.xml
macs2 stalls at the first test and pygenoetracs at test 17
Expected behavior
Screenshots
Additional context
Might be the cause of the tests timing out in the tests of #13039 here galaxyproject/tools-iuc#4318
The text was updated successfully, but these errors were encountered: