Azure-Samples / MqttApplicationSamples

Samples implementing common PubSub patterns for Edge and Cloud Brokers
MIT License
25 stars 25 forks source link

Python shows Deprecation Warning for TLS #72

Closed rido-min closed 10 months ago

rido-min commented 11 months ago

This issue is for a: (mark with an x)

- [X] bug report -> please search issues before submitting
- [ ] feature request
- [ ] documentation issue or request
- [ ] regression (a behavior that used to work and stopped in a new release)

Minimal steps to reproduce

Run the getting_started scenario for python

Any log messages given by the failure

/workspaces/MqttApplicationSamples/scenarios/getting_started/python/getting_started.py:117: DeprecationWarning: ssl.PROTOCOL_TLSv1_2 is deprecated

Expected/desired behavior

As this repo offers guidance to use existing MQTT clients, we should make sure we are providing the best sample code. We should avoid any warning.

Language

Python

OS and Version?

CodeSpaces

rido-min commented 11 months ago

this warning can be fixed by replacing

https://github.com/Azure-Samples/MqttApplicationSamples/blob/a5ce9d4b6fd7bee8bd1d21a953e248f69dbacaa6/scenarios/getting_started/python/getting_started.py#L117

with

context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
context.minimum_version = ssl.TLSVersion.TLSv1_2
context.maximum_version = ssl.TLSVersion.TLSv1_3

I was going to submit a PR, and then I realized that line is copied in all the other samples.

We should refactor the code to avoid duplication by providing common functions in /mqttclients/python so we can fix this bug by updating just one file.

cartertinney commented 11 months ago

This fix can't be centralized until #79. I could make this change manually in each place as well. Let me know.

rido-min commented 11 months ago

This fix can't be centralized until #79. I could make this change manually in each place as well. Let me know.

Why? In the same way we have ConnectionSettings we can centralize how to configure the TLS connection. What's blocking?

cartertinney commented 11 months ago

I have made a PR in #80 to suppress the warning.

With regard to centralizing Python more, this will need to be a separate item.