sync icon indicating copy to clipboard operation
sync copied to clipboard

Remove Lock class dependency from Election class

Open vbmade2000 opened this issue 7 years ago • 4 comments

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 avatar Jan 06 '18 14:01 vbmade2000

@brendandburns Any thoughts on this issue ?

vbmade2000 avatar Jan 09 '18 17:01 vbmade2000

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

Many thanks!

brendandburns avatar Jan 11 '18 03:01 brendandburns

(and sorry for the delay)

brendandburns avatar Jan 11 '18 03:01 brendandburns

@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)

vbmade2000 avatar Jan 18 '18 12:01 vbmade2000