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

[24.1] Fix possible CircularDependencyError when importing collections #19005

Conversation

davelopez
Copy link
Contributor

Fixes #18927

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?

License

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

@github-actions github-actions bot added this to the 24.1 milestone Oct 16, 2024
@davelopez davelopez changed the title Fix possible CircularDependencyError when importing collections [24.1] Fix possible CircularDependencyError when importing collections Oct 16, 2024
…nship view only

This avoids potential circular dependency errors.
@davelopez davelopez force-pushed the 24.1_fix_circular_dependency_importing_collections branch from 994787c to 81ea92a Compare October 16, 2024 13:13
@davelopez
Copy link
Contributor Author

Nope, other tests will fail if set those relationships to view-only. Out of ideas then...

@jdavcs do you know how to get rid of the circular dependency here?

@davelopez davelopez closed this Oct 16, 2024
@mvdbeek
Copy link
Member

mvdbeek commented Oct 16, 2024

Which tests are that ?

@davelopez
Copy link
Contributor Author

At least these 2:

def test_export_copied_collection():

def test_extract_copied_mapping_from_history(self, history_id):

@mvdbeek
Copy link
Member

mvdbeek commented Oct 16, 2024

I would ignore the unit test, that's fairly artificial. Do you have a link to the CI results ?

@davelopez
Copy link
Contributor Author

@mvdbeek
Copy link
Member

mvdbeek commented Oct 16, 2024

https://github.com/galaxyproject/galaxy/actions/runs/11364836955/job/31611696527#step:10:1167:

galaxy.web.framework.decorators ERROR 2024-10-16 12:46:10,011 [pN:main,p:2686,tN:WSGI_0] Uncaught exception in exposed API method:
Traceback (most recent call last):
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/web/framework/decorators.py", line 346, in decorator
    rval = func(self, trans, *args, **kwargs)
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/webapps/galaxy/api/workflows.py", line 293, in create
    stored_workflow = extract_workflow(
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/workflow/extract.py", line 48, in extract_workflow
    steps = extract_steps(
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/workflow/extract.py", line 142, in extract_steps
    raise AssertionError("Attempt to create workflow with job not connected to current history")

this might be good and I wouldn't necessarily say you're on the wrong track ?

@davelopez
Copy link
Contributor Author

davelopez commented Oct 17, 2024

this might be good and I wouldn't necessarily say you're on the wrong track ?

Alright, I'll reopen and investigate the test.

Update: GitHub doesn't let me reopen it because I force-pushed it while it was closed. I'll have to open a new one.

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.

2 participants