Add a lock for folks using threading. Mucho helpful. #121
Conversation
I can't see any *downside* to including a lock here. Basically, it just says that two threads running in parallel can't touch the code between the lock.acquire() <--> lock.release() statements concurrently. We use it in low-level util functions to prevent concurrent file reads/writes. This was a bug that showed up while running our auto-documentation examples.
|
@etpinard , @chriddyp . This came up while running a bazillion Plotly requests concurrently for generating auto-documentation. Fix works like a charm. Also, threading is built-in so we don't need to add it as a requirement. Also, I can't see how an exception can be thrown between the |
| @@ -39,8 +44,10 @@ def save_json_dict(filename, json_dict): | |||
| """Will error if filename is not appropriate, but it's checked elsewhere. | |||
| """ | |||
| if isinstance(json_dict, dict): | |||
| lock.acquire() | |||
| with open(filename, "w") as f: | |||
There was a problem hiding this comment.
whoops. we assume this directory is accessible here. I'm pretty sure there's a check for this higher-up, but that's not obvious and should be changed.
| for fn in [CREDENTIALS_FILE, CONFIG_FILE]: | ||
| utils.ensure_file_exists(fn) |
There was a problem hiding this comment.
this is clearer IMO since we're loading from it below.
|
👯 |
|
two dancers! my lucky day! |
Add a lock for folks using threading. Mucho helpful.
I can't see any downside to including a lock here. Basically, it
just says that two threads running in parallel can't touch the code
between the lock.acquire() <--> lock.release() statements
concurrently. We use it in low-level util functions to prevent
concurrent file reads/writes.
This was a bug that showed up while running our auto-documentation
examples.