sopel icon indicating copy to clipboard operation
sopel copied to clipboard

config: Eliminate side-effect of `FilenameAttribute.parse()`

Open dgw opened this issue 2 years ago • 0 comments

It took me a while, but I got around to this:

Now that FilenameAttribute creating the file/directory it's set to has been shoved in my face, I'm not sure I like that behavior. I'll add that to my list of things to think about, and write up an issue if I still don't like it after investigating.

And we're still doing it:

https://github.com/sopel-irc/sopel/blob/9a5e84d9578710cda07e815a417102a92fcf3412/sopel/config/types.py#L742-L753

There are a couple of options that I know of:

  • Use os.access() — which can fail to accurately reflect actual permissions for more exotic file locations (Python docs call out network drives as an example)
  • Use tempfile.NamedTemporaryFile — which still actually creates a file in the given location, but at least cleans up after itself
    • Some control is available over the filename's prefix and suffix, but it will always have a random component.

Why?

If the file or directory specified in a FilenameAttribute already exists, permissions aren't checked. If the file or directory doesn't exist, it will be created and left empty just because Sopel parsed its settings. That seems silly.

Let's always validate permissions, if we're going to check them; and if not, deprecate the current behavior & encourage plugin developers to use EAFP.

dgw avatar Jun 08 '23 18:06 dgw