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

[23.1] Fix DataResult type #17639

Merged
merged 1 commit into from Mar 8, 2024

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Mar 8, 2024

None isn't valid for List[Any].
Fixes:

ValidationError

1 validation error for DataResult
data
  Input should be a valid list [type=list_type, input_value=None, input_type=NoneType]
    For further information visit https://errors.pydantic.dev/2.5/v/list_type

from https://sentry.galaxyproject.org/share/issue/ed6da4c9029649cc9cc4b9a30e5144fe/

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

None isn't valid for `List[Any]`.
Fixes:
```
ValidationError

1 validation error for DataResult
data
  Input should be a valid list [type=list_type, input_value=None, input_type=NoneType]
    For further information visit https://errors.pydantic.dev/2.5/v/list_type
```
@github-actions github-actions bot added this to the 23.2 milestone Mar 8, 2024
Copy link
Contributor

@davelopez davelopez left a comment

Choose a reason for hiding this comment

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

Ouch... I wonder why mypy didn't complain about this... 🤔
Sorry about that.

@mvdbeek mvdbeek merged commit a3c14ea into galaxyproject:release_23.1 Mar 8, 2024
35 of 39 checks passed
@martenson martenson modified the milestones: 23.2, 24.0 Mar 8, 2024
@ahmedhamidawan
Copy link
Member

It is still possible to get this error:

File "./galaxy/lib/galaxy/webapps/galaxy/services/datasets.py", line 910, in _data
    stats = indexer.get_data(chrom, low, high, stats=True)
AttributeError: 'NoneType' object has no attribute 'get_data'

in case

indexer = self.data_provider_registry.get_data_provider(trans, original_dataset=dataset, source="index")

produces indexer = None
Is that expected behavior or should we be considering that case?

@mvdbeek
Copy link
Member Author

mvdbeek commented Mar 14, 2024

I don't know, what's the stack trace ? It's probably a good idea to raise a RequestInvalidException or something like this if there's no indexer for a given dataset.

@ahmedhamidawan
Copy link
Member

For the api call:

/api/datasets/f158df71bb77bb86?data_type=data&chrom=tb_ncbiRefSeq&low=0&high=50

You get:

Traceback (most recent call last):
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/uvicorn/protocols/http/h11_impl.py", line 412, in run_asgi
    |     result = await app(  # type: ignore[func-returns-value]
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 84, in __call__
    |     return await self.app(scope, receive, send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/fastapi/applications.py", line 1054, in __call__
    |     await super().__call__(scope, receive, send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/applications.py", line 123, in __call__
    |     await self.middleware_stack(scope, receive, send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 186, in __call__
    |     raise exc
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 164, in __call__
    |     await self.app(scope, receive, _send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette_context/middleware/raw_middleware.py", line 92, in __call__
    |     await self.app(scope, receive, send_wrapper)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/middleware/base.py", line 189, in __call__
    |     with collapse_excgroups():
    |   File "/usr/lib/python3.10/contextlib.py", line 153, in __exit__
    |     self.gen.throw(typ, value, traceback)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/_utils.py", line 93, in collapse_excgroups
    |     raise exc
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/middleware/base.py", line 191, in __call__
    |     response = await self.dispatch_func(request, call_next)
    |   File "/home/ahmedhamidawan/repos/galaxy/lib/galaxy/webapps/galaxy/fast_app.py", line 108, in add_x_frame_options
    |     response = await call_next(request)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/middleware/base.py", line 165, in call_next
    |     raise app_exc
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/middleware/base.py", line 151, in coro
    |     await self.app(scope, receive_or_disconnect, send_no_error)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 62, in __call__
    |     await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    |     raise exc
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    |     await app(scope, receive, sender)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/routing.py", line 758, in __call__
    |     await self.middleware_stack(scope, receive, send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/routing.py", line 778, in app
    |     await route.handle(scope, receive, send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/routing.py", line 299, in handle
    |     await self.app(scope, receive, send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/routing.py", line 79, in app
    |     await wrap_app_handling_exceptions(app, request)(scope, receive, send)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    |     raise exc
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    |     await app(scope, receive, sender)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/routing.py", line 74, in app
    |     response = await func(request)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/fastapi/routing.py", line 278, in app
    |     raw_response = await run_endpoint_function(
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/fastapi/routing.py", line 193, in run_endpoint_function
    |     return await run_in_threadpool(dependant.call, **values)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/starlette/concurrency.py", line 42, in run_in_threadpool
    |     return await anyio.to_thread.run_sync(func, *args)
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/anyio/to_thread.py", line 56, in run_sync
    |     return await get_async_backend().run_sync_in_worker_thread(
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 2144, in run_sync_in_worker_thread
    |     return await future
    |   File "/home/ahmedhamidawan/repos/galaxy/.venv/lib/python3.10/site-packages/anyio/_backends/_asyncio.py", line 851, in run
    |     result = context.run(func, *args)
    |   File "/home/ahmedhamidawan/repos/galaxy/lib/galaxy/webapps/galaxy/api/datasets.py", line 445, in show
    |     return self.service.show(trans, dataset_id, hda_ldda, serialization_params, data_type, **extra_params)
    |   File "/home/ahmedhamidawan/repos/galaxy/lib/galaxy/webapps/galaxy/services/datasets.py", line 381, in show
    |     rval = self._data(trans, dataset, **extra_params)
    |   File "/home/ahmedhamidawan/repos/galaxy/lib/galaxy/webapps/galaxy/services/datasets.py", line 910, in _data
    |     stats = indexer.get_data(chrom, low, high, stats=True)
    | AttributeError: 'NoneType' object has no attribute 'get_data'

I think at the very least, we should raise the RequestInvalidException or something similar like you said instead of it just being an Internal Server Error. I can do that in a PR if needed?

@mvdbeek
Copy link
Member Author

mvdbeek commented Mar 14, 2024

Sounds good to me, thank you!

ahmedhamidawan added a commit to ahmedhamidawan/galaxy that referenced this pull request Mar 14, 2024
In case a dataset does not have an indexer, raise an exception.
Fixes the bug mentioned in galaxyproject#17639 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants