search

SvenskaSpel / locust-plugins

A set of useful plugins/extensions for Locust
Apache License 2.0
554 stars 138 forks source link

[MQTT] locust-plugins is incompatible with paho-mqtt >= 2 #180

Closed flbraun closed 2 months ago

flbraun commented 5 months ago

After a long time of inactivity the Paho team has released a new major version for the Paho MQTT Python library (release notes, migration guide).
This major release changes the expected signature of callbacks (e.g. on_connect) which breaks locust-plugin's MqttUser.

I see three options to proceed:

  1. Pin paho-mqtt to >=1.5.0,<2 .
  2. Patch MqttUser to use the original - but now deprecated - callback signature.
  3. Adapt callback signatures in MqttUser to use the new style. This may lead to breaking changes if users customized MqttUser.

Personally, I prefer option 3. The new release of Paho brought a lot of under-the-hood-improvements the Locust ecosystem would benefit from in the long run. I'm willing to provide a PR once we agreed on a solution.

cyberw commented 5 months ago

Option 3 sounds best, PR welcome!

github-actions[bot] commented 3 months ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.

emgarten commented 3 months ago

A workaround for this is to copy the mqtt user to your own code base, then add the following line per the migration guide referenced above.

callback_api_version=mqtt.CallbackAPIVersion.VERSION1

All code stays the same but it will now work with Paho 2.0.0

flbraun commented 3 months ago

Sorry for my inactivity on the topic, I was very busy.

In the meantime, the Paho team has released version 2.1.0 which changes the default callback api version from 2 back to 1 due to backlash. Since this is the new default I think we should pin Paho to >=2.1.0. This way users of locust-plugins can still benefit from the 2.x improvements and we don't have to introduce a breaking change for users who customized MqttUser.

flbraun commented 2 months ago

@cyberw Thanks for merging the PR. Could we get a new release, please? I'd consider the issue done after that.