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.

Sign up for GitHub

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

Payload option for Channel.join() function #136

Closed
wants to merge 1 commit into from
Closed

Payload option for Channel.join() function #136

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions realtime/channel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,27 @@ def __init__(self, socket: Socket, topic: str, params: Dict[str, Any] = {}) -> N
self.listeners: List[CallbackListener] = []
self.joined = False

def join(self) -> Channel:
def join(self, payload: Dict[str, Any] = {}) -> Channel:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (code_clarification): Consider documenting the default behavior when 'payload' is not provided.

It might be helpful for users of this method to understand what the default payload behavior entails, especially if it affects the channel's state or interaction with the server.

"""
Wrapper for async def _join() to expose a non-async interface
Essentially gets the only event loop and attempt joining a topic
:param payload: Optional, additional payload, which is sent to the
server, when a channel is joined.
Comment on lines +43 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (code_clarification): Clarify the impact of the 'payload' on the server's response or channel state.

Expanding on how the payload affects the server's handling of the join request or the state of the channel could provide clearer guidance to developers using this method.

Suggested change
:param payload: Optional, additional payload, which is sent to the
server, when a channel is joined.
:param payload: Optional, additional payload, which is sent to the
server when a channel is joined. This payload can be used
to pass extra data or preferences that might influence how
the server processes the join request or manages the channel's
state. For example, it could specify initial settings or
roles for the user in the channel.

:return: Channel
"""
loop = asyncio.get_event_loop() # TODO: replace with get_running_loop
loop.run_until_complete(self._join())
loop.run_until_complete(self._join(payload))
return self

async def _join(self) -> None:
async def _join(self, payload: Dict[str, Any]) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (code_refinement): Parameter 'payload' in '_join' should be optional to match the public 'join' method.

Making 'payload' optional in '_join' ensures that calls without a payload do not fail and maintains consistency with the public interface.

Suggested change
async def _join(self, payload: Dict[str, Any]) -> None:
async def _join(self, payload: Optional[Dict[str, Any]] = None) -> None:

"""
Coroutine that attempts to join Phoenix Realtime server via a certain topic
:param payload: Optional, additional payload, which is sent to the
server, when a channel is joined.
:return: None
"""
join_req = dict(topic=self.topic, event="phx_join",
payload={}, ref=None)
payload=payload, ref=None)

try:
await self.socket.ws_connection.send(json.dumps(join_req))
Expand Down