support ubuntu24.04 with python3.12 #1964
Conversation
|
Getting corporate approval |
|
I have checked with my legal team. |
|
@microsoft-github-policy-service agree |
|
@nkuchta @D1v38om83r Hi, this is really important for us as ubuntu 24.04 blocked from using azure monitor features. |
| @@ -39,6 +39,7 @@ | |||
| import hashlib | |||
| import fileinput | |||
| import contextlib | |||
| import distro | |||
There was a problem hiding this comment.
Sorry for the drive-by comments @marcurdy - I don't work on this project but I would like to get this issue resolved so my team can use Ubuntu 24.04 hosts on Azure without issue!
distro is a third-party package, and from what I can see in the way that this AzureMonitorAgent project is set up, the project has no requirements.txt / setup.py or any other mechanism to declare third-party dependencies. So I think this is intended to be deployed as just a single script, and adding a third-party dep will not work?
There was a problem hiding this comment.
on my ubuntu 24.04, distro comes ootb even in minimized installations. IMHO dropping it would be wrong, but it would be prudent to put it behind a try clause.
There was a problem hiding this comment.
@gilbahat cool, good to know. a try/except sounds appropriate here. When I get a chance this week I can do some tests with Docker to see whether major distributions include this library out of the box.
| @@ -1651,12 +1652,11 @@ def find_vm_distro(operation): | |||
| # platform commands used below aren't available after Python 3.6 | |||
| if sys.version_info < (3,7): | |||
| try: | |||
| vm_dist, vm_ver, vm_id = platform.linux_distribution() | |||
| vm_dist = distro.id() | |||
There was a problem hiding this comment.
since you're adding fixes for Python 3.12 and higher, I'd leave this branch alone that was meant for Python <3.7
| @@ -1651,12 +1652,11 @@ def find_vm_distro(operation): | |||
| # platform commands used below aren't available after Python 3.6 | |||
There was a problem hiding this comment.
This comment and check seem wrong actually - platform.dist and platform.linux_distribution were documented in Python 3.7 and removed in Python 3.8.
| @@ -47,6 +47,7 @@ import zipfile | |||
| import json | |||
| import datetime | |||
| import xml.sax.saxutils | |||
| import distro | |||
There was a problem hiding this comment.
Same comment as above - waagent seems to be set up to be a single file script, so I don't think third-party dependencies would be allowed
Supporting Python3.12 requires adding "r" for raw in regexes and replacing platform for module distro. This change needs to be tested on FreeBSD. Additionally, to allow ubuntu 24.04 to work, the supported distros list was expanded.