Skip to content

Try to open file in UploadCompletionHook initializer with given path to ensure file upload works#4390

Merged
aabmass merged 9 commits intoopen-telemetry:mainfrom
DylanRussell:test_file_exists
Apr 2, 2026
Merged

Try to open file in UploadCompletionHook initializer with given path to ensure file upload works#4390
aabmass merged 9 commits intoopen-telemetry:mainfrom
DylanRussell:test_file_exists

Conversation

@DylanRussell
Copy link
Copy Markdown
Contributor

@DylanRussell DylanRussell commented Apr 2, 2026

Description

Make a change to try to open a file in UploadCompletionHook initializer with the given path in the given format to ensure file upload works, if it fails raise an error...

The current implementation doesn't check the path, lets a bad path get passed in, and then when the upload hook is invoked we repeatedly fail.. Better to check if things are working upfront and throw a more understandable error..

Type of change

Please delete options that are not relevant.

  • [x ] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Unit tests.. Tested locally by running the following code:

import fsspec, posixpath
_format="jsonl"
_content_type = (
            "application/json"
            if _format == "json"
            else "application/jsonl"
        )
base_path = "gs://my-bucket-name"
_fs, base_path = fsspec.url_to_fs(base_path)
base_path = _fs.unstrip_protocol(base_path)
test_path = posixpath.join(
    base_path,
    f".one_off_test_to_see_if_upload_works.{_format}",
)

with _fs.open(
    test_path, "w", content_type=_content_type
) as file:
  file.write("\n")
_fs.rm_file(test_path)

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • [X ] No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • [ X] Followed the style guidelines of this project
  • [ X] Changelogs have been updated
  • [ X] Unit tests have been added
  • [ X] Documentation has been updated

@aabmass aabmass merged commit 048b4ad into open-telemetry:main Apr 2, 2026
860 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants