-
Notifications
You must be signed in to change notification settings - Fork 81
Add mute feature #197
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
Add mute feature #197
Conversation
- Updated the mute functionality with the active flag, excludedStream and excludedStreamIds - Added a new test in test_opentok.py
opentok/opentok.py
Outdated
@@ -1422,30 +1424,45 @@ def __init__( | |||
|
|||
|
|||
|
|||
def mute(self, session_id: str, stream_id: str= "", options: dict = {}) -> requests.Response: | |||
def mute(self, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@geekchick Let's change this to two methods: mute_all(session_id, active, excludedStreamIds)
and mute_stream(session_id, stream_id
.
What is the data
parameter? Can we remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jeffswartz the data parameter is the body or payload for the request. It gets defined as a dictionary in the method signature and used in the code to populate the request body.
- Split the ForceMute into two functions: mute_all and mute_stream - added return statements for the urls in the endpoints.py file - added a new test for the single stream
opentok/opentok.py
Outdated
|
||
|
||
|
||
def mute_stream(self, session_id: str, stream_id: str, options: dict = {}) -> requests.Response: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment about the options
parameter of the mute_all()
method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, will do!
opentok/opentok.py
Outdated
session_id: str, | ||
excludedStreamIds: Optional[List[str]], | ||
active: bool= True, | ||
options: dict = {}) -> requests.Response: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this parameter? We do not include this options parameter in other methods. Can the method use a private variable internally that populates the options? I don't think we should include a parameter that the developer will not use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jeffswartz ! Sure, I can definitely remove the options parameter in the function.
- Removed options from mute_all - Removed body from mute_stream
-For this Jira ticket