-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Fix race conditions #1498
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 race conditions #1498
Conversation
Also, bare except: would trap KeyboardInterrupt, which is usually not intended.
Thanks for digging into this @pb-cdunn, I haven't run into this before but your reasoning and these changes both make sense so I'm happy to merge this.
Regarding warnings, my intention in #1195 was to only raise a warning if a user explicitly asks to perform an operation that needs write access to the PLOTLY_DIR directory, and this write cannot be performed.
This is a "Heisenbug", a race condition. You might not be able to reproduce it, but I'll describe it anyway.
We're running this as an automated test, as a user we cannot control, so use
export PLOTLY_DIR=whateverre: #1076, addressed in #1195
This is still a problem with
plotly-3.7.1. Inplotly/files.py:Lines
30and33can fail, wrongly, if two processes run this at the same time. (f.write()is fine here, IMO.)Note that I want to make warnings into errors in my tests. (
pytest -W error), so this is a big problem for me even if you simply trap andwarn().I'm not sure if there is still a warning in 3.7.1, but like I said, it's a Heisenbug. I can only spend so much time on this. For me, it was happening via
pytest-xdist, in a case where I lacked permissions to create~/.plotly.PLOTLY_DIRlets me reduce the likelihood of error, but the race conditions remain.