livekit / python-sdks

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

Update ffi to reflect proto2 syntax changes #290

Closed lukasIO closed 4 weeks ago

lukasIO commented 4 weeks ago

@theomonnom any idea why the eventemitter test might be failing? This PR doesn't include any changes around eventemitter 🤔

theomonnom commented 4 weeks ago

@theomonnom any idea why the eventemitter test might be failing? This PR doesn't include any changes around eventemitter 🤔

That's so weird, it is passing on main, will take a look

bcherry commented 3 weeks ago

@lukasIO @theomonnom I see a similar issue here as in Node - a runtime error about missing required fields when running the basic_room example:

(venv) @13:15:32 ~/dev/livekit/python-sdks/examples [main]: test-creds-local python basic_room.py
/Users/ben/dev/livekit/python-sdks/examples/basic_room.py:154: DeprecationWarning: There is no current event loop
  loop = asyncio.get_event_loop()
ERROR:asyncio:Task exception was never retrieved
future: <Task finished name='Task-1' coro=<main() done, defined at /Users/ben/dev/livekit/python-sdks/examples/basic_room.py:12> exception=EncodeError('Message livekit.proto.FfiRequest is missing required fields: connect.options.adaptive_stream,connect.options.join_retries')>
Traceback (most recent call last):
  File "/Users/ben/dev/livekit/python-sdks/examples/basic_room.py", line 140, in main
    await room.connect(os.getenv("LIVEKIT_URL"), token)
  File "/Users/ben/dev/livekit/python-sdks/livekit-rtc/livekit/rtc/room.py", line 364, in connect
    resp = FfiClient.instance.request(req)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ben/dev/livekit/python-sdks/livekit-rtc/livekit/rtc/_ffi_client.py", line 219, in request
    proto_data = req.SerializeToString()
                 ^^^^^^^^^^^^^^^^^^^^^^^
google.protobuf.message.EncodeError: Message livekit.proto.FfiRequest is missing required fields: connect.options.adaptive_stream,connect.options.join_retries
INFO:livekit:livekit_ffi::server:137:livekit_ffi::server - initializing ffi server v0.11.3
INFO:livekit:livekit_ffi::cabi:27:livekit_ffi::cabi - initializing ffi server v0.11.3
lukasIO commented 3 weeks ago

When implementing the proto2 syntax I made all fields required that weren't explicitly marked as optional before. Maybe this also overdid things for the RoomOptions? Not sure if these missing fields should really be required for the FFI request, wdyt @theomonnom ?