-
-
Notifications
You must be signed in to change notification settings - Fork 329
Fix permissions issue with DirectoryStore #493
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
Fix permissions issue with DirectoryStore #493
Conversation
cc @jakirkham @alimanfoo for review when you have a moment cc @eddienko @d-v-b if possible, could you confirm these changes fix the permissions issues you've encountered recently |
@jrbourbeau some quick tests on my end (creating a container and inspecting permissions) suggests that your changes do indeed fix the problem! Thanks for putting this together. |
Awesome, thank you for testing! |
Looks good to me as well. I am not sure the test is needed now. |
Thanks @jrbourbeau! Also thanks everyone for testing. 😄 |
Adding my thanks @jrbourbeau and @eddienko, this one has been a real pain point, great to see this go in. |
This PR builds on #445 and uses the built-in
open
function for creating temporary files inDirectoryStore
instead oftempfile.NamedTemporaryFile
. As outlined in #325 (comment), the reasons for doing this are to avoid the limited permissions associated withtempfile.NamedTemporaryFile
while also not needing to query currentumask
and then usechmod
. It's also worth noting that here we're usinguuid
to generate a unique filepath for temporary files that are created.Closes #325
TODO:
tox -e docs
)