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 logging in export_history.py, fix Azure and S3 tests #19542

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

kysrpex
Copy link
Contributor

@kysrpex kysrpex commented Feb 5, 2025

TL;DR Absolute non-issue, really irrelevant fix, just noticed things were wrong while fixing something else.

On line 59 from export_history.py, actual_uri = _write_to_destination(options.file_sources, os.path.abspath(out_file), destination_uri), but _write_to_destination() yields just the path of the saved file, so destination_uri != actual_uri is always True and the print statement always kicks-in.

In addition, write_from() from test/unit/files/_util.py is wrong in a way such that assert_can_write_and_read_to_conf() from the same file should always fail. It went unnoticed just because the tests that use it are skipped by default.

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 export_history.py from the command line to export a history using any file source that does not choose a path for uploaded files (see Let file sources choose a path for uploaded files #19154, at the moment any file source except eLabFTW will do).
    2. Run Azure or S3 tests from the files that are changed in this PR.

License

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

On line 59 from export_history.py, `actual_uri = _write_to_destination(options.file_sources, os.path.abspath(out_file), destination_uri)`, but `_write_to_destination()` yields just the path of the saved file, so `destination_uri != actual_uri` is always `True` and the print statement always kicks-in.

In addition, `write_from()` from test/unit/files/_util.py is wrong in a way such that `assert_can_write_and_read_to_conf()` from the same file should always fail. It went unnoticed just because the tests that use it are skipped by default.
Comment on lines -2662 to 2665
file_source_uri = urlparse(str(self.file_source_uri))
file_source_path = self.file_sources.get_file_source_path(self.file_source_uri)
file_source = file_source_path.file_source
assert os.path.exists(self.out_file)
self.file_source_uri = f"{file_source_uri.scheme}://{file_source_uri.netloc}" + file_source.write_from(
self.file_source_uri = f"{file_source.get_scheme()}://{file_source.get_prefix()}" + file_source.write_from(
file_source_path.path, self.out_file, user_context=self.user_context
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't have any effect.

@kysrpex kysrpex self-assigned this Feb 5, 2025
@github-actions github-actions bot added this to the 25.0 milestone Feb 5, 2025
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.

1 participant