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

Replace os.rename with os.replace #3322

Merged
merged 1 commit into from
Nov 22, 2024
Merged

Conversation

yyyyuki
Copy link

@yyyyuki yyyyuki commented Nov 22, 2024

Description

Fix #2975.
Replace os.rename with os.replace.

Motivation and Context

According to the Python documentation, os.rename should be avoided for cross-platform compatibility. It is recommended to use os.replace().
This change ensures that the file move operation works correctly across different filesystems.

On a Windows environment, running the following program twice will result in an error.

with local_target.LocalTarget('data.txt').open("w") as f:
    f.write("hello")

FileExistsError: [WinError 183] Cannot create a file when that file already exists

Have you tested this? If so, how?

Tested locally.

@yyyyuki yyyyuki requested review from dlstadther and a team as code owners November 22, 2024 07:00
@dlstadther dlstadther merged commit b1d3ca3 into spotify:master Nov 22, 2024
48 checks passed
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.

local_target.py - os.rename change to os.replace ?
2 participants