Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Made S3Bucket._resolve_path Windows-friendly #192

Merged
merged 3 commits into from
Jan 18, 2023
Merged

Made S3Bucket._resolve_path Windows-friendly #192

merged 3 commits into from
Jan 18, 2023

Conversation

dominictarro
Copy link
Contributor

@dominictarro dominictarro commented Jan 6, 2023

Read/write from Windows OS produced object keys with backslashes and no folder hierarchy. This was due to pathlib inferring backslashes on Windows when joining bucket_path. Backslashes are frowned upon in the AWS Object key naming guidelines.

Explicitly made Path join bucket_path POSIX path string.

Closes

Example

Screenshots

Checklist

  • References any related issue by including "Closes #" or "Closes ".
    • If no issue exists and your change is not a small fix, please create an issue first.
  • Includes tests or only affects documentation.
  • Passes pre-commit checks.
    • Run pre-commit install && pre-commit run --all locally for formatting and linting.
  • Includes screenshots of documentation updates.
    • Run mkdocs serve view documentation locally.
  • Summarizes PR's changes in CHANGELOG.md

Read/write from Windows OS produced object keys with backslashes and no folder hierarchy. This was due to `pathlib` inferring backslashes on Windows when joining `bucket_path`. Backslashes are frowned upon in the [AWS Object key naming](https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html) guidelines.

Explicitly made `Path` join `bucket_path` POSIX path string.
@ahuang11
Copy link
Contributor

ahuang11 commented Jan 6, 2023

Thank you for contributing! Do you mind fixing other calls to str()?

For example:

await self.write_path(str(remote_file_path), content=local_file_content)

return str(Path(self.bucket_folder) / bucket_path)

And more

Also can you add a unit test around here?
https://github.com/PrefectHQ/prefect-aws/blob/main/tests/test_s3.py#L575

When object keys are sent to S3 or are shown in logs, it is always in POSIX form.

'\' is not recognized as a delimiter on POSIX systems. Paths containing '\' will not be converted to '/' unless passed as a `PureWindowsPath` object. Windows systems always recognize and convert '\' to '/'.
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Andrew <15331990+ahuang11@users.noreply.github.com>
@ahuang11 ahuang11 merged commit d7ba1cb into PrefectHQ:main Jan 18, 2023
@ahuang11
Copy link
Contributor

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants