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

Fix wrong URI written to ExportObjectResultMetadata when exporting histories to eLabFTW #19541

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

kysrpex
Copy link
Contributor

@kysrpex kysrpex commented Feb 5, 2025

Fix history exports to eLabFTW having a wrong URI saved to ExportObjectResultMetadata.uri, which results in not being able to reimport the histories.

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. Run eLabFTW, for example using Docker Compose (or use https://demo.elabftw.net).
    2. Copy the configuration samples from file_sources_conf.yml.sample and user_preferences_extra_conf.yml.sample to your own configuration files.
    3. Create an account and generate an API Key on eLabFTW (or use the demo account from https://demo.elabftw.net).
    4. Configure the endpoint and API Key on the user preferences page.
    5. Create an experiment or resource in eLabFTW.
    6. Export a history to eLabFTW as an attachment of the created experiment or resource.
    7. Reimport the history.

License

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

…histories to eLabFTW

Fix history exports to eLabFTW having a wrong URI saved to `ExportObjectResultMetadata.uri`, which results in not being able to reimport the histories.
@kysrpex kysrpex requested a review from davelopez February 5, 2025 11:01
@github-actions github-actions bot added this to the 25.0 milestone Feb 5, 2025
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.

Thank you!

@kysrpex
Copy link
Contributor Author

kysrpex commented Feb 5, 2025

As usual, I do not merge stuff because I cannot. I thought I do not have permission, but it turns out I could merge if tests worked 😄

grafik

@davelopez
Copy link
Contributor

davelopez commented Feb 5, 2025

Hah!, right, there are some legit failures here 😅

https://github.com/galaxyproject/galaxy/actions/runs/13155893235/job/36712840840?pr=19541#step:7:5910

@kysrpex
Copy link
Contributor Author

kysrpex commented Feb 6, 2025

The new version of pysam 0.23.0 released yesterday is the source of the failures :( To prevent them, the bot would also have to pin dependencies for the individual packages somehow.

@davelopez
Copy link
Contributor

Ahh! I see! Thanks for checking, some of the errors were suspiciously related to file sources 😅

Traceback (most recent call last):
    File "/tmp/gxpkgtestenvqXrIPA/lib/python3.13/site-packages/galaxy/files/sources/__init__.py", line 540, in realize_to
      self._realize_to(source_path, native_path, user_context, opts=opts)
      ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  OSError: [Errno 9] Bad file descriptor

But I see now that they are also failing in other PRs

@kysrpex
Copy link
Contributor Author

kysrpex commented Feb 6, 2025

Ahh! I see! Thanks for checking, some of the errors were suspiciously related to file sources 😅

Traceback (most recent call last):
    File "/tmp/gxpkgtestenvqXrIPA/lib/python3.13/site-packages/galaxy/files/sources/__init__.py", line 540, in realize_to
      self._realize_to(source_path, native_path, user_context, opts=opts)
      ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  OSError: [Errno 9] Bad file descriptor

But I see now that they are also failing in other PRs

Yes, I also saw these errors when I opened the PR for the initial implementation. Counterintuitively, they don't seem to be a bug haha.

@davelopez davelopez merged commit c7cce09 into galaxyproject:dev Feb 6, 2025
50 of 56 checks passed
Copy link

github-actions bot commented Feb 6, 2025

This PR was merged without a "kind/" label, please correct.

@kysrpex kysrpex self-assigned this Feb 6, 2025
@kysrpex kysrpex deleted the fix_elabftw_history_exports branch February 6, 2025 15:58
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.

3 participants