eclipse-velocitas / vehicle-app-python-sdk

vehicle-app-python-sdk
Apache License 2.0
7 stars 18 forks source link

[Bug]: Velocitas Python template-based app middleware initialisation fails #118

Closed daniel-schaubach-beg closed 6 months ago

daniel-schaubach-beg commented 6 months ago

Severity

Blocker

What release version, tag or commit-hash did you use?

v0.13.0

Current Behavior

During startup of a Velocitas Python Template based app, the following error occurs {"stream":"stderr","log":" File \"\u003cfrozen importlib._bootstrap\u003e\", line 1006, in _find_and_load_unlocked\n","time":"2024-02-15T11:15:26.165212Z"} {"stream":"stderr","log":" File \"\u003cfrozen importlib._bootstrap\u003e\", line 688, in _load_unlocked\n","time":"2024-02-15T11:15:26.165270625Z"} {"stream":"stderr","log":" File \"PyInstaller/loader/pyimod02_importers.py\", line 352, in exec_module\n","time":"2024-02-15T11:15:26.165298375Z"} {"stream":"stderr","log":" File \"sdv/config.py\", line 49, in \u003cmodule\u003e\n","time":"2024-02-15T11:15:26.16534975Z"} {"stream":"stderr","log":" File \"sdv/config.py\", line 36, in __init__\n","time":"2024-02-15T11:15:26.165594875Z"} {"stream":"stderr","log":" File \"sdv/config.py\", line 41, in __create_middleware\n","time":"2024-02-15T11:15:26.1660935Z"} {"stream":"stderr","log":" File \"sdv/native/middleware.py\", line 34, in __init__\n","time":"2024-02-15T11:15:26.16616375Z"} {"stream":"stderr","log":" File \"sdv/native/mqtt.py\", line 42, in __init__\n","time":"2024-02-15T11:15:26.16621375Z"} {"stream":"stderr","log":"TypeError: Client.__init__() missing 1 required positional argument: 'callback_api_version'\n","time":"2024-02-15T11:15:26.166349125Z"} {"stream":"stderr","log":"[9] Failed to execute script 'main' due to unhandled exception!\n","time":"2024-02-15T11:15:26.16640025Z"} Seems to be due to https://stackoverflow.com/questions/77984857/paho-mqtt-unsupported-callback-api-version-error https://eclipse.dev/paho/files/paho.mqtt.python/html/migrations.html

Indeed, the Velocitas app is build using paho_mqtt-2.0.0 Output from Github Action: `#11 134.1 Requirement already satisfied: dapr>=1.6.0 in /usr/local/lib/python3.10/site-packages (from sdv==0.9.2->-r ./app/requirements-links.txt (line 1)) (1.8.3)

11 134.4 Collecting paho-mqtt>=1.6.1

11 134.6 Downloading paho_mqtt-2.0.0-py3-none-any.whl (66 kB)

11 134.6 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 66.9/66.9 kB 7.3 MB/s eta 0:00:00

11 134.8 Collecting opentelemetry-distro<=0.36b0

11 134.8 Downloading opentelemetry_distro-0.36b0-py3-none-any.whl (3.3 kB)

11 135.0 Collecting opentelemetry-instrumentation-logging<=0.36b0`

Steps to Reproduce

Build a new app based on the Velocitas Python Template The app builds fine but will include paho_mqtt-2.0.0 which requires an additional parameter for client creation and thus causes an error in sdv/native/mqtt.py

Expected Behavior

Middleware should initialize correctly

Possible Solution

https://stackoverflow.com/questions/77984857/paho-mqtt-unsupported-callback-api-version-error https://eclipse.dev/paho/files/paho.mqtt.python/html/migrations.html

Add Version argument here? https://github.com/eclipse-velocitas/vehicle-app-python-sdk/blob/de93359c0edd4a085630199ed6913244188ff915/velocitas_sdk/native/mqtt.py#L42

Fix version to pre 2.0 here? https://github.com/eclipse-velocitas/vehicle-app-python-sdk/blob/de93359c0edd4a085630199ed6913244188ff915/setup.py#L23

Additional Information

No response

Code of Conduct

eriksven commented 6 months ago

We ran into the same issue. For now, my preference would be to add the version argument like described in the migration guide: mqttc = mqtt.Client(mqtt.CallbackAPIVersion.VERSION1). A follow up issue would be to upgrade to the behavior of the VERSION2.

@doosuu @MP91 What do you think?

MP91 commented 6 months ago

@daniel-schaubach-beg @eriksven At the first glance both solutions would be possible. Currently I am more in favor of pinning the client version to something <2, since we know this is working. We did't had the time to do a sorrow investigation about version 2, though.