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 copying metadata to copied job outputs #17007

Merged

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Nov 10, 2023

Fixes #17003

(Please replace this header with a description of your pull request. Please include BOTH what you did and why you made the changes. The "why" may simply be citing a relevant Galaxy issue.)
(If fixing a bug, please add any relevant error or traceback)
(For UI components, it is recommended to include screenshots or screencasts)

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.

@mvdbeek mvdbeek changed the base branch from dev to release_23.1 November 10, 2023 14:15
@mvdbeek mvdbeek changed the title Fix copying metadata to copied job outputs [23.1] Fix copying metadata to copied job outputs Nov 10, 2023
@github-actions github-actions bot added area/testing area/database Galaxy's database or data access layer area/testing/integration labels Nov 10, 2023
@mvdbeek mvdbeek force-pushed the fix_copied_outputs_metadata branch from 7cc50db to 824d346 Compare November 10, 2023 14:37
@@ -667,7 +667,6 @@ def from_external_value(self, value, parent, path_rewriter=None):
# directory. Correct.
file_name = path_rewriter(file_name)
mf.update_from_file(file_name)
os.unlink(file_name)
Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't the right place to do this, cleaning up the job working directory will handle this correctly. fixes the directory metadata strategy case

Copy link
Member Author

Choose a reason for hiding this comment

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

I bet this is also what happened in #14087

@mvdbeek mvdbeek marked this pull request as ready for review November 10, 2023 14:38
@github-actions github-actions bot added this to the 23.2 milestone Nov 10, 2023
@mvdbeek mvdbeek requested a review from a team November 10, 2023 14:39
@bgruening
Copy link
Member

FAILED test/integration/objectstore/test_remote_objectstore_cache_operations.py::TestCacheOperation::test_cache_repopulated_metadata_file - AssertionError: assert 3 == 2

That one might be related.

@mvdbeek mvdbeek force-pushed the fix_copied_outputs_metadata branch from 824d346 to eb3802e Compare November 10, 2023 16:57
@mvdbeek
Copy link
Member Author

mvdbeek commented Nov 10, 2023

That one might be related.

yep, fixed in a rebase

@mvdbeek mvdbeek force-pushed the fix_copied_outputs_metadata branch from eb3802e to 2fc1b64 Compare November 10, 2023 17:03
@bgruening bgruening merged commit 643b0d4 into galaxyproject:release_23.1 Nov 10, 2023
38 checks passed
@bgruening
Copy link
Member

Merci!

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.

Unavailable metadata after extracting non-terminal dataset from collection
2 participants