livekit / python-sdks

LiveKit real-time and server SDKs for Python
https://docs.livekit.io
Apache License 2.0
74 stars 24 forks source link

Confusing Argument Naming in `participant.py` #127

Closed Liamazing closed 6 months ago

Liamazing commented 6 months ago

Just spent a little while debugging an issue and root causing it to what I think is a confusing argument name, though forgive me that I do not have a broad understanding of the codebase.

The signature of LocalParticipant.unpublish_track follows:

    async def unpublish_track(self, track_sid: str) -> None:

Given this signature, I was under the impression that I would want to use the sid of the track supplied to the corresponding call to LocalParticipant.publish_track. As it turns out, it is not the Track.sid that satisfies this argument, but the TrackPublication.sid. In fact, the Track.sid and TrackPublication.track.sid will both be TR_unknown, which is not useful. Perhaps there would be a minor improvement in readability with track_publication_sid instead of track_sid as the argument name?

theomonnom commented 6 months ago

track_publication_sid must be the same as track_sid. The problem is that the track_sid is not updated after publishing. I'm going to fix that in a PR