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

Remove Lock class dependency from Election class #11

Open
vbmade2000 opened this issue Jan 6, 2018 · 4 comments
Open

Remove Lock class dependency from Election class #11

vbmade2000 opened this issue Jan 6, 2018 · 4 comments

Comments

@vbmade2000
Copy link
Contributor

There is an instantiation of Lock class inside Election class. This makes Election class dependent on Lock class. You can see instantiation at https://github.com/metaparticle-io/sync/blob/master/python/metaparticle_sync/election.py#L8:21.

IMHO, instance of Lock class should be supplied from outside. It removes dependency of Election class on Lock class. Also I believe it makes Election class more unit test friendly.

Let me know your thoughts. If you agree, I can send a PR.

@vbmade2000
Copy link
Contributor Author

@brendandburns Any thoughts on this issue ?

@brendandburns
Copy link
Contributor

Sure, that works for me, happy to see a PR.

Many thanks!

@brendandburns
Copy link
Contributor

(and sorry for the delay)

@vbmade2000
Copy link
Contributor Author

@brendandburns I am confused about lock_callback and lock_lost_callback constructor arguments as their default value are from Lock class itself. If We supply Lock instance from outside of Election class, I am not sure how to handle these two arguments.

self.lock = Lock(name, lock_callback=self._lock, lock_lost_callback=self._lost_lock)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants