search

kquick / Thespian

Python Actor concurrency library
MIT License
189 stars 24 forks source link

globalName as Actor instance attribute #17

Closed waqaraqeel closed 6 years ago

waqaraqeel commented 6 years ago

globalName value passed to the createActor() call will be available in the actor instance as self.globalName

@kquick Old fork got all mixed up. New PR.

kquick commented 6 years ago

Hi @waqaraqeel Just confirming that I've seen this and although I won't have time to review this today, I expect to review it and get back to you within 24 hours. I appreciate your contribution, thank you! -Kevin

kquick commented 6 years ago

Thanks again for submitting this, @waqaraqeel ! I made a couple of post-merge adjustments, mostly to shift the self.globalName from an init parameter to be handled the same as the self.myAddress, but it was really nice to leverage the work you did getting this implemented.

Best regards, Kevin

waqaraqeel commented 6 years ago

@kquick Thank you for the nice comments. This is my first OSS contribution. I don't understand how globalName is being assigned to the instance in case of multiproc systems after your new commits. Could you please explain?

kquick commented 6 years ago

It's a bit subtle: the Actor object uses a property to defer the lookup to the underlying _myRef object (https://github.com/kquick/Thespian/blob/master/thespian/actors.py#L167-L171), which is either the simpleSystemBase where it's set here (https://github.com/kquick/Thespian/blob/master/thespian/system/simpleSystemBase.py#L495), or a multi-process base, where the _myRef refers to an ActorManager object (set here https://github.com/kquick/Thespian/blob/master/thespian/system/actorManager.py#L76) which has the globalName (https://github.com/kquick/Thespian/blob/master/thespian/system/actorManager.py#L40). The latter path is tricky because it passes through the creation of a new process and a subsequent instantiation of the Actor in the new process.

waqaraqeel commented 6 years ago

Got it. Thanks!