-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Simplify lock acquire #5695
Simplify lock acquire #5695
Conversation
Currently we are acquiring 3 locks. We should only have 2 (we can't have only one because of readthedocs#5672)
I still want to test this more locally before merging :) |
Locking is working, this is ready for review. |
Why not to acquire the lock immediately inside the Otherwise, the |
That's the trick, the lock needs to be acquired inside the |
Yeah, but we can create the |
There are two build environments, that's why we need two locks. build_env_1 -> lock Nesting isn't possible due how build_env works and expects lock to be inside the build env context manager. Also, the second build_env depends on the first, which gets the config file to initialize the next build_env with the correct docker image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes a lot more sense 💯
Currently we are acquiring 3 locks.
We should only have 2 (we can't have only one because of #5672)
Basically I'm removing the locks from
setup_python_env
andbuild_docs
to only acquire the lock in a central place (run_build
)