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

[DOM-58567] Datasets Snapshot File Upload: Handle Windows-style Paths #203

Merged

Conversation

ddl-eric-jin
Copy link
Contributor

@ddl-eric-jin ddl-eric-jin commented Jun 27, 2024

Link to JIRA

DOM-59086

What issue does this pull request solve?

Uploading files to a Dataset on a windows can fail since the upload API expects a Unix-style filepath. The backslash will be interpreted as an escape character.

What is the solution?

When running python-domino on windows, transform the path to a Unix-style path, which the API to send an upload chunk for Datasets expects.

Testing

Tested domino.datasets_upload_files for a file located at "\test\test.txt" and added unit tests.

  • Unit test(s)

Pull Request Reminders

@ddl-eric-jin ddl-eric-jin requested a review from a team as a code owner June 27, 2024 22:29
@ddl-eric-jin ddl-eric-jin self-assigned this Jun 27, 2024
@ddl-eric-jin ddl-eric-jin requested review from a team and ddl-giuliocapolino June 27, 2024 22:30
Copy link
Contributor

@ddl-giuliocapolino ddl-giuliocapolino left a comment

Choose a reason for hiding this comment

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

interesting - i’d have assumed that os.path.relpath would use os.path under the hood. but LGTM!

@ddl-olsonJD
Copy link
Collaborator

Please update change logs, and complete the What is the solution? section

@ddl-olsonJD ddl-olsonJD self-requested a review July 2, 2024 17:46
@ddl-eric-jin ddl-eric-jin merged commit a7c73ec into master Jul 2, 2024
@ddl-eric-jin ddl-eric-jin deleted the ddl-eric-jin.DOM-58567.fe-single-file-dataset-download-fix branch July 2, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants