镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Issue in fs.move.move_file overwrite logic in fs==2.4.16 #535

Description

@AleksMat

Today's release of fs==2.4.16 introduced an code-breaking change that isn't really documented in the release notes and might be a bug.

Here is the code sample that works for fs<=2.4.15 but doesn't work for fs==2.4.16:

import fs.move
from fs.osfs import OSFS


source_fs = OSFS("source-folder")
target_fs = OSFS("target-folder")


FILENAME = "data.txt"
for filesystem in [source_fs, target_fs]:
    with filesystem.open(FILENAME, "w") as fp:
        fp.write("")

fs.move.move_file(source_fs, FILENAME, target_fs, FILENAME)

Basically, we have 2 different OSFS filesystem objects, a file in both, and we want to move a version of the file from filesystem one to another. For fs==2.4.16 this raises an overwrite error permission:

---------------------------------------------------------------------------
DestinationExists                         Traceback (most recent call last)
Input In [1], in <module>
     11     with filesystem.open(FILENAME, "w") as fp:
     12         fp.write("")
---> 14 fs.move.move_file(source_fs, FILENAME, target_fs, FILENAME)

File ~/.local/share/virtualenvs/xxx/lib/python3.8/site-packages/fs/move.py:85, in move_file(src_fs, src_path, dst_fs, dst_path, preserve_time, cleanup_dst_on_error)
     83         with _src_fs.lock(), _dst_fs.lock():
     84             with OSFS(common) as base:
---> 85                 base.move(rel_src, rel_dst, preserve_time=preserve_time)
     86                 return  # optimization worked, exit early
     87 except ValueError:
     88     # This is raised if we cannot find a common base folder.
     89     # In this case just fall through to the standard method.

File ~/.local/share/virtualenvs/xxx/lib/python3.8/site-packages/fs/base.py:1161, in FS.move(self, src_path, dst_path, overwrite, preserve_time)
   1140 """Move a file from ``src_path`` to ``dst_path``.
   1141 
   1142 Arguments:
   (...)
   1158 
   1159 """
   1160 if not overwrite and self.exists(dst_path):
-> 1161     raise errors.DestinationExists(dst_path)
   1162 if self.getinfo(src_path).is_dir:
   1163     raise errors.FileExpected(src_path)

DestinationExists: destination '/target-folder/data.txt' exists

I don't know if this change was intentional but there is at least an inconsistency with the following example that works for both fs<=2.4.15 and fs==2.4.16

import fs.move
from fs.osfs import OSFS


filesystem = OSFS(".")

FILENAME = "data.txt"
source_path = fs.path.join("source-folder", FILENAME)
target_path = fs.path.join("target-folder", FILENAME)

for path in [source_path, target_path]:
    with filesystem.open(path, "w") as fp:
        fp.write("")

fs.move.move_file(filesystem, source_path, filesystem, target_path)

In this example we have 1 filesystem object, a file in both folders, and we again copy the file from one folder to another.

This inconsistency comes down to the following lines of code:

  • here overwrite flag is set to True,
  • here it is not - the source of the problem and potentially a bug?

So I would suggest that:

  • If this was an unintentional change then the missing flag overwrite=True is added.
  • If this was an intentional change then an overwrite flag parameter is added to move_file function so that users can specify themselves which option they prefer. For my use case I would really like to specify overwrite=True so that I don't have to write additional checks.

Let me know if you would need any help with this issue.

Activity

  1. althonos commented on May 2, 2022

    @althonos
    Member

    Hi @AleksMat ,

    thanks for the very prompt issue ticket. The change was likely introduced as a side-effect of #523. I think for backward compatibility we should at least add overwrite=True, but it may be worth thinking about whether adding a parameter to move_file would be worth it.

    @tfeldmann, care to have a look?

  2. tfeldmann commented on May 5, 2022

    @tfeldmann
    Contributor

    Sorry I'm on vacation and cannot get to this until next month

  3. althonos commented on May 5, 2022

    @althonos
    Member

    No problem, i'll have a look first if you can't 😉

  4. added a commit that references this issue on Jul 7, 2022
    6763644
  5. tfeldmann commented on Jul 7, 2022

    @tfeldmann
    Contributor

    I submitted a PR for this issue.

    I think having an overwrite argument in move_file would be nice. The only thing I don't like is that in FS.move(...) overwrite defaults to False and in move.move_file we would have to default to True for backwards compatibility reasons?

  6. lurch commented on Jul 8, 2022

    @lurch
    Contributor

    Yeah that's unfortunate, but I think that maintaining backwards-compatibility has to be the primary concern here? We can always attempt to make API-defaults more consistent when we move to PyFS v3?

  7. tfeldmann commented on Jul 8, 2022

    @tfeldmann
    Contributor

    Is v3 already planned or in development somewhere?

  8. althonos commented on Jul 8, 2022

    @althonos
    Member

    We have a conversation in the Pyfilesystem maintainers organization listing some ideas and changes, but nothing has started yet, would be good to have a branch going at some point I agree.

  9. added a commit that references this issue on Sep 28, 2025
    f84055e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions