Unverified Commit 37bc4540 authored by mvdbeek's avatar mvdbeek
Browse files

Don't commit without having seta hid

hid-less items in a history are bad, and this is potentially one way to
produce them.

We've got a bunch of them in the history and this has been showing up in
as https://sentry.galaxyproject.org/share/issue/3cc69eedc9f841b6ae8ac02e3f88e8fe/:

```
ValidationError: 210 validation errors for HistoryContentsWithStatsResult
contents.0.HDACustom.type
  Input should be 'file' [type=literal_error, input_value='collection', input_type=str]
    For further information visit https://errors.pydantic.dev/2.5/v/literal_error
contents.0.HDACustom.history_content_type
  Input should be 'dataset' [type=literal_error, input_value='dataset_collection', input_type=str]
    For further information visit https://errors.pydantic.dev/2.5/v/literal_error
contents.0.HDADetailed.hid
  Input should be a valid integer [type=int_type, input_value=None, input_type=NoneType]
    For further information visit https://errors.pydantic.dev/2.5/v/int_type
contents.0.HDADetailed.type
  Input should be 'file' [type=literal_error, input_value='collection', input_type=str]
    For further information visit https://errors.pydantic.dev/2.5/v/literal_error
contents.0.HDADetailed.history_content_type
  Input should be 'dataset' [type=literal_error, input_value='dataset_collection', input_type=str]
    For fur...
  File "starlette/applications.py", line 123, in __call__
    await self.middleware_stack(scope, receive, send)
  File "starlette/middleware/errors.py", line 186, in __call__
    raise exc
  File "starlette/middleware/errors.py", line 164, in __call__
    await self.app(scope, receive, _send)
  File "starlette_context/middleware/raw_middleware.py", line 92, in __call__
    await self.app(scope, receive, send_wrapper)
  File "starlette/middleware/base.py", line 189, in __call__
    with collapse_excgroups():
  File "contextlib.py", line 155, in __exit__
    self.gen.throw(typ, value, traceback)
  File "starlette/_utils.py", line 93, in collapse_excgroups
    raise exc
  File "starlette/middleware/base.py", line 191, in __call__
    response = await self.dispatch_func(request, call_next)
  File "galaxy/webapps/galaxy/fast_app.py", line 108, in add_x_frame_options
    response = await call_next(request)
  File "starlette/middleware/base.py", line 165, in call_next
    raise app_exc
  File "starlette/middleware/base.py", line 151, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "starlette/middleware/exceptions.py", line 62, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "starlette/routing.py", line 758, in __call__
    await self.middleware_stack(scope, receive, send)
  File "starlette/routing.py", line 778, in app
    await route.handle(scope, receive, send)
  File "starlette/routing.py", line 299, in handle
    await self.app(scope, receive, send)
  File "starlette/routing.py", line 79, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "starlette/routing.py", line 74, in app
    response = await func(request)
  File "fastapi/routing.py", line 278, in app
    raw_response = await run_endpoint_function(
  File "fastapi/routing.py", line 193, in run_endpoint_function
    return await run_in_threadpool(dependant.call, **values)
  File "starlette/concurrency.py", line 42, in run_in_threadpool
    return await anyio.to_thread.run_sync(func, *args)
  File "anyio/to_thread.py", line 56, in run_sync
    return await get_async_backend().run_sync_in_worker_thread(
  File "anyio/_backends/_asyncio.py", line 2144, in run_sync_in_worker_thread
    return await future
  File "anyio/_backends/_asyncio.py", line 851, in run
    result = context.run(func, *args)
  File "galaxy/webapps/galaxy/api/history_contents.py", line 465, in index
    items = self.service.index(
  File "galaxy/webapps/galaxy/services/history_contents.py", line 312, in index
    return self.__index_v2(trans, history_id, params, serialization_params, filter_query_params, accept)
  File "galaxy/webapps/galaxy/services/history_contents.py", line 1006, in __index_v2
    return HistoryContentsWithStatsResult(contents=items, stats={"total_matches": total_matches})
```

I'm hesitatant to relax the validation here, these are really pretty bad
bugs. We do appear to have test coverage here via https://app.codecov.io/gh/galaxyproject/galaxy/commit/4a4ead2b5ee913b61a97b84b60e77f28415d5ad9/blob/lib/galaxy/tools/actions/upload_common.py#L144
parent 33538bea
Loading
Loading
Loading
Loading
+0 −2
Original line number Diff line number Diff line
@@ -140,8 +140,6 @@ def __new_history_upload(trans, uploaded_dataset, history=None, state=None):
        hda.state = state
    else:
        hda.state = hda.states.QUEUED
    with transaction(trans.sa_session):
        trans.sa_session.commit()
    history.add_dataset(hda, genome_build=uploaded_dataset.dbkey, quota=False)
    permissions = trans.app.security_agent.history_get_default_permissions(history)
    trans.app.security_agent.set_all_dataset_permissions(hda.dataset, permissions, new=True, flush=False)