Presence engine - states and events #178

seba-aln · 2024-01-03T13:07:37Z

feat: Optional Event Engine for Subscribe Loop

Xavrax

LGTB 2 comments

pubnub/event_engine/manage_effects.py

pubnub/pubnub_asyncio.py

pubnub/event_engine/manage_effects.py

pubnub/event_engine/effects.py

jguz-pubnub · 2024-02-06T11:25:26Z

pubnub/event_engine/dispatcher.py

-    def dispatch_effect(self, effect: effects.PNEffect):
-        if not self._managed_effects_factory:
-            self._managed_effects_factory = manage_effects.ManagedEffectFactory(self._pubnub, self._event_engine)
+    def dispatch_effect(self, invocation: invocations.PNInvocation):


Mhm, I don't think we should differentiate Effects that emit status (or anything else) from other non-cancellable Effects. They should have the same interface or method to fire them. I know it's not possible to fix it, so let's keep this comment so that others are aware of this difference.

I will look at it

jguz-pubnub · 2024-02-06T11:28:37Z

pubnub/event_engine/effects.py

+        if self.invocation.region:
+            request.region(self.invocation.region)
+
+        if feature_enabled('PN_MAINTAIN_PRESENCE_STATE') and hasattr(self.pubnub, 'state_container'):


I'm curious, why do you need a check like this?

hasattr(self.pubnub, 'state_container')

Isn't the internal state_container variable always present in PubNub?

no it's not the internal variable for PubNub. It's only in PubNubAsyncio because it's only used there. This may be redundant check. I'll verify it

pubnub/event_engine/effects.py

jguz-pubnub · 2024-02-06T11:34:58Z

pubnub/event_engine/effects.py

+class HeartbeatDelayedEffect(Effect):
+    def __init__(self, pubnub_instance, event_engine_instance,
+                 invocation: Union[invocations.PNManageableInvocation, invocations.PNCancelInvocation]) -> None:
+        super().__init__(pubnub_instance, event_engine_instance, invocation)


The same, let's think about improving it while working on a new reconnection policy

jguz-pubnub · 2024-02-06T11:36:49Z

pubnub/event_engine/effects.py

+        return self._managed_invocations[invocation.__class__.__name__](self._pubnub, self._event_engine, invocation)
+
+
+class EmitEffect:


There should be two separate Effects for emitting messages and status, respectively. Let's leave it if this requires significant amount of work

Xavrax

LGTM - some nitpickings and questions

Xavrax · 2024-02-06T11:27:59Z

pubnub/event_engine/containers.py

+            self.channel_states[channel] = state
+
+    def get_state(self, channels: list):
+        return {**self.get_channels_states(channels)}


Q: What exactly meant double wyłuskanie here?

it's a dict extraction. Also it's redundant. I'll fix it :)

Xavrax · 2024-02-06T11:30:18Z

pubnub/event_engine/effects.py

+    pubnub: PubNub = None
+    event_engine = None
+    invocation: Union[invocations.PNManageableInvocation, invocations.PNCancelInvocation]
+    stop_event = None
+    logger: logging.Logger
+    task: asyncio.Task


Nitpicking:
It is easy to make a mistake when looking at the default values.

Suggested change

pubnub: PubNub = None

event_engine = None

invocation: Union[invocations.PNManageableInvocation, invocations.PNCancelInvocation]

stop_event = None

logger: logging.Logger

task: asyncio.Task

pubnub: PubNub = None

event_engine: StateMachine = None

stop_event: PNEvent = None

invocation: Union[invocations.PNManageableInvocation, invocations.PNCancelInvocation]

logger: logging.Logger

task: asyncio.Task

Xavrax · 2024-02-06T11:32:53Z

pubnub/event_engine/effects.py

+        tt = self.invocation.timetoken or 0
+        if hasattr(self.pubnub, 'event_loop'):
+            self.stop_event = self.get_new_stop_event()
+            self.run_async(self.handshake_async(channels=channels,
+                                                groups=groups,
+                                                timetoken=tt,
+                                                stop_event=self.stop_event))
+
+    async def handshake_async(self, channels, groups, stop_event, timetoken: int = 0):
+        request = Subscribe(self.pubnub).channels(channels).channel_groups(groups).cancellation_event(stop_event)
+
+        if feature_enabled('PN_MAINTAIN_PRESENCE_STATE') and hasattr(self.pubnub, 'state_container'):
+            state_container = self.pubnub.state_container
+            request.state(state_container.get_state(channels))
+
+        request.timetoken(0)


Do you need to set the timetoken to zero in each of the occurrences? It would be nice to have a single point of truth in case of code-reviewing-and-debugging.
In C# there was so many of the reassignments like that and it caused the bug. :/

Xavrax · 2024-02-06T11:33:22Z

pubnub/event_engine/effects.py

+    def give_up(self, reason: PubNubException, attempt: int, timetoken: int = 0):
+        self.logger.error(f"GiveUp called on Unspecific event. Reason: {reason}, Attempt: {attempt} TT:{timetoken}")
+        raise PubNubException('Unspecified Invocation')
+
+    def failure(self, reason: PubNubException, attempt: int, timetoken: int = 0):
+        self.logger.error(f"Failure called on Unspecific event. Reason: {reason}, Attempt: {attempt} TT:{timetoken}")
+        raise PubNubException('Unspecified Invocation')


same as above?

Xavrax · 2024-02-06T11:35:39Z

pubnub/event_engine/effects.py

+        except asyncio.CancelledError:
+            pass


It only catches the CancelledError. Is it intended?

jguz-pubnub · 2024-02-06T11:44:45Z

pubnub/event_engine/models/states.py


    def on_exit(self):
        super().on_exit()
-        return effects.CancelHandshakeReconnectEffect()
+        return invocations.CancelHandshakeReconnectInvocation()

    def disconnect(self, event: events.DisconnectEvent, context: PNContext) -> PNTransition:


IMHO all transitions should be part of Transition function that was mentioned in the State Machine Architecture decision. This shouldn't be the logic of any State. Let's keep this comment so that others are aware of this difference.

jguz-pubnub

Approved. However, some areas might differ in terms of decisions/design in comparison to other SDKs. Let's skip it for now if Presence & Subscribe works as expected and let's think about any adjustments/improvements in the future.

Xavrax · 2024-02-08T09:36:09Z

examples/cli_chat.py

+        print(f"{presence.event} {presence.uuid}\n")
+
+    def status(self, pubnub, status):
+        # print(status.__dict__)


seba-aln · 2024-02-08T10:12:24Z

pubnub/enums.py

@@ -19,7 +19,7 @@ def string(cls, method):
            return "PATCH"


-class PNStatusCategory(object):
+class PNStatusCategory(Enum):


Disclaimer: For most of situations this doesn't change anything. Enum inherits from object (which is leftover from py2.7 btw) and only adds functionality like this status.category.name to extract the name based on the value

seba-aln · 2024-02-08T11:38:54Z

@pubnub-release-bot release

pubnub-release-bot · 2024-02-08T11:50:25Z

🚀 Release successfully completed 🚀

seba-aln requested review from jguz-pubnub and wkal-pubnub as code owners January 3, 2024 13:07

seba-aln force-pushed the event-engine/presence branch from 7c6d680 to 2dc14cb Compare January 3, 2024 14:10

Presence engine - states and events

4e9eb3e

seba-aln force-pushed the event-engine/presence branch from 2dc14cb to 4e9eb3e Compare January 3, 2024 14:16

seba-aln added 2 commits January 9, 2024 13:14

Fixes and effects

f294766

Add missing presence_enable switch

bc977cc

seba-aln force-pushed the event-engine/presence branch from ca5b2fb to bc977cc Compare January 9, 2024 14:19

Xavrax approved these changes Jan 9, 2024

View reviewed changes

pubnub/event_engine/manage_effects.py Outdated Show resolved Hide resolved

pubnub/pubnub_asyncio.py Show resolved Hide resolved

seba-aln added 2 commits January 26, 2024 15:05

Add ee param to heartbeat, subscribe and leave endpoints

be47efe

Presence engine, refactors and everything else including meaning of life

9693ef1

seba-aln force-pushed the event-engine/presence branch from 74cdb09 to 9693ef1 Compare January 26, 2024 14:21

seba-aln added 6 commits January 30, 2024 13:19

Add pnpres to channel groups

34c310b

rename delayed effects

08aa63d

Removed code repetitions + improved behave tests

3a7f67e

Fixed Heartbeat Retry

f235268

Fixes

eb9cb49

Rename effects to invocations

1103ba8

seba-aln force-pushed the event-engine/presence branch 2 times, most recently from 54b367b to 3ce8c86 Compare February 5, 2024 21:08

Add support for subscription state

ce317a6

seba-aln force-pushed the event-engine/presence branch from 3ce8c86 to ce317a6 Compare February 5, 2024 21:22

jguz-pubnub reviewed Feb 6, 2024

View reviewed changes

pubnub/event_engine/manage_effects.py Outdated Show resolved Hide resolved

jguz-pubnub reviewed Feb 6, 2024

View reviewed changes

pubnub/event_engine/effects.py Show resolved Hide resolved

jguz-pubnub reviewed Feb 6, 2024

View reviewed changes

pubnub/event_engine/effects.py Show resolved Hide resolved

jguz-pubnub reviewed Feb 6, 2024

View reviewed changes

pubnub/event_engine/effects.py Show resolved Hide resolved

jguz-pubnub reviewed Feb 6, 2024

View reviewed changes

Xavrax approved these changes Feb 6, 2024

View reviewed changes

jguz-pubnub reviewed Feb 6, 2024

View reviewed changes

jguz-pubnub self-requested a review February 6, 2024 11:48

jguz-pubnub approved these changes Feb 6, 2024

View reviewed changes

Fix leaving channels

462f1cb

seba-aln force-pushed the event-engine/presence branch from 0b66f04 to 462f1cb Compare February 7, 2024 21:41

Added example

89b442e

Xavrax approved these changes Feb 8, 2024

View reviewed changes

Fixes in example

93453ad

seba-aln commented Feb 8, 2024

View reviewed changes

PubNub SDK v7.4.0 release.

96cc36b

seba-aln merged commit 4c03ecd into master Feb 8, 2024
9 checks passed

seba-aln deleted the event-engine/presence branch February 8, 2024 11:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Presence engine - states and events #178

Presence engine - states and events #178

seba-aln commented Jan 3, 2024 •

edited

Loading

Xavrax left a comment

jguz-pubnub Feb 6, 2024 •

edited

Loading

seba-aln Feb 6, 2024

jguz-pubnub Feb 6, 2024 •

edited

Loading

seba-aln Feb 6, 2024

jguz-pubnub Feb 6, 2024

jguz-pubnub Feb 6, 2024

Xavrax left a comment

Xavrax Feb 6, 2024

seba-aln Feb 6, 2024

Xavrax Feb 6, 2024

Xavrax Feb 6, 2024

Xavrax Feb 6, 2024

Xavrax Feb 6, 2024

jguz-pubnub Feb 6, 2024

jguz-pubnub left a comment

Xavrax Feb 8, 2024

seba-aln Feb 8, 2024

seba-aln commented Feb 8, 2024

pubnub-release-bot commented Feb 8, 2024

		return self._managed_invocations[invocation.__class__.__name__](self._pubnub, self._event_engine, invocation)


		class EmitEffect:

Presence engine - states and events #178

Presence engine - states and events #178

Conversation

seba-aln commented Jan 3, 2024 • edited Loading

Xavrax left a comment

Choose a reason for hiding this comment

jguz-pubnub Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jguz-pubnub Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xavrax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jguz-pubnub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seba-aln commented Feb 8, 2024

pubnub-release-bot commented Feb 8, 2024

seba-aln commented Jan 3, 2024 •

edited

Loading

jguz-pubnub Feb 6, 2024 •

edited

Loading

jguz-pubnub Feb 6, 2024 •

edited

Loading