[24.1] Fix possible CircularDependencyError when importing collections #19017

davelopez · 2024-10-17T07:27:09Z

Fixes #18927
xref #19005

Making copied HistoryDatasetCollectionAssociation relationships view-only as an attempt to avoid potential circular dependency errors.

In practice, this gets rid of the error when importing but I'm not 100% sure this is the correct solution if copied_from_history_dataset_collection_association and/or copied_to_history_dataset_collection_association are meant to be used for persistency and not just for querying at any point.

How to test the changes?

I've included appropriate automated tests.
This is a refactoring of components with existing test coverage.
Instructions for manual testing are as follows:
- Try to import any exported invocation provided in Cannot import RO-Crate of an Invocation using Collection as input #18927
- Observe there is no CircularDependencyError during the import

License

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

… only This avoids potential circular dependency errors.

To respect the view-only flag for copied_from_history_dataset_collection_association relationship.

davelopez · 2024-10-17T11:22:13Z

I think e14d76a will fix these tests https://github.com/galaxyproject/galaxy/actions/runs/11380325900?pr=19017

But I'm not sure what will happen with this piece of code:

galaxy/lib/galaxy/model/store/__init__.py

Lines 1000 to 1011 in 1a6b7fe

    
           hdca_copied_from_sinks = object_import_tracker.hdca_copied_from_sinks 
        
           if copied_from_object_key in object_import_tracker.hdcas_by_key: 
        
               hdca.copied_from_history_dataset_collection_association = object_import_tracker.hdcas_by_key[ 
        
                   copied_from_object_key 
        
               ] 
        
           else: 
        
               if copied_from_object_key in hdca_copied_from_sinks: 
        
                   hdca.copied_from_history_dataset_collection_association = object_import_tracker.hdcas_by_key[ 
        
                       hdca_copied_from_sinks[copied_from_object_key] 
        
                   ] 
        
               else: 
        
                   hdca_copied_from_sinks[copied_from_object_key] = dataset_collection_key

Here we cannot associate directly using the id because those are new objects (their id is still None) and having the viewonly flag to true will ignore this assignment as far as I understand.

…port

davelopez · 2024-10-17T13:56:12Z

Here we cannot associate directly using the id because those are new objects (their id is still None) and having the viewonly flag to true will ignore this assignment as far as I understand.

307840a will fix this, but it feels "dirty" to commit in the middle of the import... so probably a bad idea? I am not sure how to work around it though... 😞

Update: nope, this early commit breaks then the HDA import with "HistoryDatasetAssociation without hid, this is not valid":

Traceback (most recent call last):
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/uvicorn/protocols/http/h11_impl.py", line 407, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/uvicorn/middleware/proxy_headers.py", line 69, in __call__
    return await self.app(scope, receive, send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/fastapi/applications.py", line 1054, in __call__
    await super().__call__(scope, receive, send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/applications.py", line 123, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/middleware/errors.py", line 186, in __call__
    raise exc
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/middleware/errors.py", line 164, in __call__
    await self.app(scope, receive, _send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette_context/middleware/raw_middleware.py", line 92, in __call__
    await self.app(scope, receive, send_wrapper)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/middleware/base.py", line 191, in __call__
    response = await self.dispatch_func(request, call_next)
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/contextlib.py", line 131, in __exit__
    self.gen.throw(type, value, traceback)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/_utils.py", line 93, in collapse_excgroups
    raise exc
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/middleware/base.py", line 191, in __call__
    response = await self.dispatch_func(request, call_next)
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/webapps/galaxy/fast_app.py", line 109, in add_x_frame_options
    response = await call_next(request)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/middleware/base.py", line 165, in call_next
    raise app_exc
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/middleware/base.py", line 151, in coro
    await self.app(scope, receive_or_disconnect, send_no_error)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/middleware/exceptions.py", line 65, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/routing.py", line 756, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/routing.py", line 776, in app
    await route.handle(scope, receive, send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/routing.py", line 297, in handle
    await self.app(scope, receive, send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/routing.py", line 77, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/routing.py", line 72, in app
    response = await func(request)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/fastapi/routing.py", line 278, in app
    raw_response = await run_endpoint_function(
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/fastapi/routing.py", line 193, in run_endpoint_function
    return await run_in_threadpool(dependant.call, **values)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/starlette/concurrency.py", line 42, in run_in_threadpool
    return await anyio.to_thread.run_sync(func, *args)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/anyio/to_thread.py", line 56, in run_sync
    return await get_async_backend().run_sync_in_worker_thread(
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/anyio/_backends/_asyncio.py", line 2144, in run_sync_in_worker_thread
    return await future
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/anyio/_backends/_asyncio.py", line 851, in run
    result = context.run(func, *args)
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/webapps/galaxy/api/history_contents.py", line 1054, in create_from_store
    return self.service.create_from_store(trans, history_id, create_payload, serialization_params)
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/webapps/galaxy/services/history_contents.py", line 543, in create_from_store
    object_tracker = self.create_objects_from_store(
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/webapps/galaxy/services/base.py", line 167, in create_objects_from_store
    return create_objects_from_store(
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/managers/model_stores.py", line 329, in create_objects_from_store
    object_tracker = model_import_store.perform_import(
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/model/store/__init__.py", line 415, in perform_import
    self._flush()
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/model/store/__init__.py", line 1329, in _flush
    self.sa_session.commit()
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/orm/scoping.py", line 597, in commit
    return self._proxied.commit()
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/orm/session.py", line 2017, in commit
    trans.commit(_to_root=True)
  File "<string>", line 2, in commit
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/orm/state_changes.py", line 139, in _go
    ret_value = fn(self, *arg, **kw)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/orm/session.py", line 1302, in commit
    self._prepare_impl()
  File "<string>", line 2, in _prepare_impl
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/orm/state_changes.py", line 139, in _go
    ret_value = fn(self, *arg, **kw)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/orm/session.py", line 1277, in _prepare_impl
    self.session.flush()
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/orm/session.py", line 4341, in flush
    self._flush(objects)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/orm/session.py", line 4369, in _flush
    self.dispatch.before_flush(self, flush_context, objects)
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.8/site-packages/sqlalchemy/event/attr.py", line 378, in __call__
    fn(*args, **kw)
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/model/base.py", line 184, in before_flush
    obj.__strict_check_before_flush__()
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/model/__init__.py", line 5222, in __strict_check_before_flush__
    raise Exception(f"HistoryDatasetAssociation {self} without hid, this is not valid")
Exception: HistoryDatasetAssociation <galaxy.model.HistoryDatasetAssociation(None) at 0x7f2b2c09b670> without hid, this is not valid

mvdbeek · 2024-11-26T14:58:42Z

I think the problem is that we do something like self.copied_from_history_dataset_collection = self if we don't have the source of the hdca. The fix is 8f3b3b4, but I found and fixed quite some additional issues in #19207

Make HistoryDatasetCollectionAssociation copied_... relationship view…

cd7dc7a

… only This avoids potential circular dependency errors.

davelopez added kind/bug area/backend labels Oct 17, 2024

github-actions bot added this to the 24.1 milestone Oct 17, 2024

davelopez marked this pull request as draft October 17, 2024 10:55

Associate copied_from_history_dataset_collection_association by id

e14d76a

To respect the view-only flag for copied_from_history_dataset_collection_association relationship.

davelopez marked this pull request as ready for review October 17, 2024 11:22

Fix copied_from_history_dataset_collection_association handling on im…

307840a

…port

davelopez marked this pull request as draft October 17, 2024 15:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[24.1] Fix possible CircularDependencyError when importing collections #19017

[24.1] Fix possible CircularDependencyError when importing collections #19017

davelopez commented Oct 17, 2024

davelopez commented Oct 17, 2024 •

edited

Loading

davelopez commented Oct 17, 2024 •

edited

Loading

mvdbeek commented Nov 26, 2024

[24.1] Fix possible CircularDependencyError when importing collections #19017

Are you sure you want to change the base?

[24.1] Fix possible CircularDependencyError when importing collections #19017

Conversation

davelopez commented Oct 17, 2024

How to test the changes?

License

davelopez commented Oct 17, 2024 • edited Loading

davelopez commented Oct 17, 2024 • edited Loading

mvdbeek commented Nov 26, 2024

davelopez commented Oct 17, 2024 •

edited

Loading

davelopez commented Oct 17, 2024 •

edited

Loading