Skip to content
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

Config locks #2654

Merged
merged 12 commits into from Jul 23, 2019
Merged

Config locks #2654

merged 12 commits into from Jul 23, 2019

Conversation

Tobotimus
Copy link
Member

@Tobotimus Tobotimus commented May 8, 2019

This provides a way to lock by individual value, at any level of the config hierarchy desired.

Uses a weakref dictionary to maintain locks across multiple Value objects whilst they're simultaneously alive.

This also updates the async with config.foo() context manager to use the foo value's lock, unless a new acquire_lock kwarg is set to False for some edge case where acquiring the lock is undesirable.

Tobotimus added 2 commits May 8, 2019
Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
@Tobotimus Tobotimus added the Type: Feature label May 8, 2019
Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
@Tobotimus Tobotimus changed the base branch from V3/develop to V3/feature/pimp_my_config May 22, 2019
@Tobotimus Tobotimus mentioned this pull request May 22, 2019
3 tasks
@mikeshardmind mikeshardmind added this to the 3.2.0 milestone May 23, 2019
@Tobotimus Tobotimus removed V3 labels Jun 29, 2019
@Tobotimus Tobotimus changed the base branch from V3/feature/pimp_my_config to V3/develop Jul 17, 2019
Copy link
Contributor

@mikeshardmind mikeshardmind left a comment

This needs some work...

image

(This has a registered channel : channels={}, was being lazy)
(gotta love how the __getattr__ continues to bite us)

However, there's another issue as well with this:

image

@Tobotimus Tobotimus requested a review from mikeshardmind Jul 20, 2019
Copy link
Contributor

@mikeshardmind mikeshardmind left a comment

I've not found any other issues. I also looked through (did not manually test each of) the other places we already use config as a context manager to see if I could spot anywhere this might turn into a deadlock which wasn't before (though this would have indicated that section was problematic for the things this aims to help mitigate) and didn't spot any.

Definitely want to see if we can get this onto some of our larger ambassador bots without issue for testing if we have willing victims participants

@mikeshardmind mikeshardmind merged commit af096bc into Cog-Creators:V3/develop Jul 23, 2019
1 check passed
@Tobotimus Tobotimus deleted the V3/config_locks branch Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants