3
\$\begingroup\$

I'll have to move a large network directory with many sub-directories and thousands of files. A number of users work on these files on a daily basis. OS is windows.

The move itself (copy and delete) I'll do using robocopy.

Before starting the batch process of moving, I would like to check for locked files. In case some files or folders are locked, I won't bother starting the process.

robocopy has a dry run switch, but that doesn't check for locked files which would lead to an unsuccessful run.

I quickly came up with something like the below, in Python:

import os

is_locked=list()

for root, dirnames, filenames in os.walk('path/to/directory'):
    for filename in filenames:
        full_filename = os.path.join(root, filename)
        try:
            pf = open(full_filename, 'a')
            pf.close()
        except PermissionError:
            is_locked.append(full_filename)

Your opinion whether this is a good or bad way of doing it? Any general comment? Performance, etc?

Is it harmless to be opening these files?

--- edit ---

Summary of the most relevant comments:

  • Break the loop when first locked file is found.
\$\endgroup\$
7
  • \$\begingroup\$ What protocol does the network mount use? \$\endgroup\$
    – Reinderien
    Oct 28, 2019 at 21:54
  • \$\begingroup\$ Also, do you care whether your method changes the file timestamp? You probably should.. \$\endgroup\$
    – Reinderien
    Oct 28, 2019 at 21:56
  • \$\begingroup\$ This is a rather... significant change in code and functionality, which isn't really the point of Code Review. I don't know how best to proceed though... \$\endgroup\$
    – Gloweye
    Oct 29, 2019 at 8:51
  • 3
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Vogel612
    Oct 29, 2019 at 11:30
  • \$\begingroup\$ This question is being discussed on meta \$\endgroup\$
    – Vogel612
    Oct 29, 2019 at 11:33

1 Answer 1

3
\$\begingroup\$

Bugs:

  • After building your list, you throw it away without printing anything (and if not, please supply all your code)

Minor:

  • pathlib >>> os. It leads to much better readable code, and less boilerplate.
  • is_locked=list() should be: is_locked = [] Note spaces and the literal.
  • pf.close() isn't a reliable statement. Instead, consider using something like with open(full_filename) as pf: pass
  • "path/to/directory" is a magic variable. Make it a global constant instead.
  • Use a if __name__ == "__main__": guard. Always.

Are you going to manually unlock all the files?

If not, you can just break after the first error and inform the user.

\$\endgroup\$
10
  • 1
    \$\begingroup\$ Nearly the entire of your answer is useless with regard to the actual code. This is why I made my comment. Also you shouldn't rely on your reasoning with regard to pf.close(). \$\endgroup\$
    – Peilonrayz
    Oct 29, 2019 at 13:45
  • \$\begingroup\$ You should not rely on the file being closed, even with the close. Because the exception will be raised before that line of code is reached. Only with can save you from that (or in this case putting the closing into a finally block). \$\endgroup\$
    – Graipher
    Oct 29, 2019 at 13:55
  • 1
    \$\begingroup\$ Sorry, I don't remember who posted what comment earlier. But I'll update my answer to the with style, which is better indeed. \$\endgroup\$
    – Gloweye
    Oct 29, 2019 at 13:56
  • \$\begingroup\$ @Graipher I don't believe your are correct there, note how mgr = (EXPR) from PEP 343 is the same as pf = open(...), meaning that a with would do nothing that .close doesn't, in this instance. It may, however, ensure a file is closed if an error is raised between the creation and closure of the file. \$\endgroup\$
    – Peilonrayz
    Oct 29, 2019 at 16:48
  • 1
    \$\begingroup\$ @Graipher When "the open raises an exception" then with and .close are the same - neither the with or the .close are reached. Yes, I already mentioned that in my first comment - "It may, however, ensure a file is closed if an error is raised between the creation and closure of the file." \$\endgroup\$
    – Peilonrayz
    Oct 29, 2019 at 17:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.