8000 Remove destination data files if they exist. by antarcticrainforest · Pull Request #658 · pypa/flit · GitHub
[go: up one dir, main page]
Skip to content

Remove destination data files if they exist. #658

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 28, 2023
Merged

Remove destination data files if they exist. #658

merged 3 commits into from
Sep 28, 2023

Conversation

antarcticrainforest
Copy link
Contributor

We recently switched to flit and I came across this issue when I installed a package using the "symlink" mode. What I did was adding more data files and then re installing the package to create the symlinks to the recently added data files:

flit install -s
Traceback (most recent call last):
FileExistsError: [Errno 17] File exists: '/home/wilfred/workspace/deployment/assets/share/freva/deployment/config/create_tables.sql' -> '/tmp/py_311/share/freva/deployment/config/create_tables.sql'

Since without -s any existing data files would be overridden anyways I figured it's ok to delete any existing data files.

@takluyver
Copy link
Member

Hi Martin! Long time no see. How are you?

Thanks, I think this change makes sense. Just for neatness, though, let's try deleting it and ignore the error if it's not there. Pathlib makes this easy with .unlink(missing_ok=True). It won't usually make a difference, but it's one less way to hit odd behaviour if something else is adding/deleting files at the same time.

@antarcticrainforest
Copy link
Contributor Author

Hi Thomas,
all good on my end.

Yes, that makes sense what you're saying. I didn't see that you're already using pathlib in the code.

Btw. give me a shout when you're next time around in HH.

@takluyver
Copy link
Member

Thanks! I was actually just there last week - the next visit will probably be around early December, I'll let you know when I've figured it out.

Yeah, I'm wildly inconsistent on using pathlib vs os.path. 😄

takluyver
takluyver reviewed Sep 27, 2023
View reviewed changes
@antarcticrainforest @takluyver
Update flit/install.py
f1a773d 
Fix typo and .unlink method

Co-authored-by: Thomas Kluyver <takowl@gmail.com>
@takluyver
Copy link
Member

My bad, I had missed the note that missing_ok was added in Python 3.8.

I think we can drop Python 3.7 support from the main flit package now, as it's EOL anyway. I'll do a separate PR for that.

@takluyver takluyver mentioned this pull request Sep 27, 2023
Merged
@takluyver
Copy link
Member

OK, that's done now in #660. I'll merge that tomorrow if no-one objects, then we can re-run the tests on this one.

@takluyver
Copy link
Member

Closing & reopening to re-run tests against the latest commit.

@takluyver takluyver closed this Sep 28, 2023
@takluyver takluyver reopened this Sep 28, 2023
@takluyver takluyver added this to the 3.10 milestone Sep 28, 2023
@takluyver takluyver added the bug label Sep 28, 2023
@takluyver
Copy link
Member

Tests are passing now. Thanks Martin!

@takluyver takluyver merged commit a259482 into pypa:main Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@takluyver takluyver takluyver left review comments

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
3.10
Development

Successfully merging this pull request may close these issues.
2 participants
@antarcticrainforest @takluyver
0