search

tenstorrent / tt-smi

Tenstorrent console based hardware information program
Apache License 2.0
18 stars 3 forks source link

tt-smi and tt-tools-common are incompatible with pydantic 2.0+ #27

Closed afaerber closed 1 month ago

afaerber commented 1 month ago

pyproject.toml has 'pydantic==1.*'. (tt-tools-common's pyproject.toml does not list it as dependency, despite using it - separate issue.)

However, with distro-provided pydantic 2.7.3 package I am seeing errors like the following from both tt-tools-common and tt-smi code:

$ tt-smi -v
Traceback (most recent call last):
  File "/usr/bin/tt-smi", line 5, in <module>
    from tt_smi import main
  File "/usr/lib/python3.11/site-packages/tt_smi/__init__.py", line 7, in <module>
    from .tt_smi import *
  File "/usr/lib/python3.11/site-packages/tt_smi/tt_smi.py", line 29, in <module>
    from tt_tools_common.reset_common.reset_utils import (
  File "/usr/lib/python3.11/site-packages/tt_tools_common/reset_common/reset_utils.py", line 13, in <module>
    import tt_tools_common.reset_common.host_reset_log as log
  File "/usr/lib/python3.11/site-packages/tt_tools_common/reset_common/host_reset_log.py", line 144, in <module>
    @optional
     ^^^^^^^^
  File "/usr/lib/python3.11/site-packages/tt_tools_common/reset_common/host_reset_log.py", line 57, in optional
    return dec(cls)
           ^^^^^^^^
  File "/usr/lib/python3.11/site-packages/tt_tools_common/reset_common/host_reset_log.py", line 50, in dec
    _cls.__fields__[field].required = False
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'FieldInfo' object has no attribute 'required'
Traceback (most recent call last):
  File "/usr/bin/tt-smi", line 5, in <module>
    from tt_smi import main
  File "/usr/lib/python3.11/site-packages/tt_smi/__init__.py", line 7, in <module>
    from .tt_smi import *
  File "/usr/lib/python3.11/site-packages/tt_smi/tt_smi.py", line 33, in <module>
    from tt_smi.tt_smi_backend import (
  File "/usr/lib/python3.11/site-packages/tt_smi/tt_smi_backend.py", line 15, in <module>
    from tt_smi import log
  File "/usr/lib/python3.11/site-packages/tt_smi/log.py", line 148, in <module>
    @optional
     ^^^^^^^^
  File "/usr/lib/python3.11/site-packages/tt_smi/log.py", line 56, in optional
    return dec(cls)
           ^^^^^^^^
  File "/usr/lib/python3.11/site-packages/tt_smi/log.py", line 49, in dec
    _cls.__fields__[field].required = False
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'FieldInfo' object has no attribute 'required'

A workaround is to patch the following files to import pydantic.v1[.fields] rather than pydantic[.fields]: /usr/lib/python3.11/site-packages/tt_tools_common/reset_common/host_reset_log.py /usr/lib/python3.11/site-packages/tt_smi/log.py Cf. https://docs.pydantic.dev/latest/migration/#continue-using-pydantic-v1-features

A better solution would probably be to update the code to use pydantic >= 2.0. Cf. https://docs.pydantic.dev/latest/migration/#migration-guide

afaerber commented 1 month ago

Updating the various occurrences of __fields__ to model_fields did not help. According to StackOverflow the required field was indeed dropped from FieldInfo and annotations should be used instead.

The bump-pydantic tool 0.8.0 only suggested one change in tt-tools-common and nothing here for tt-smi:

[   16s] + bump-pydantic --diff tt_tools_common
[   17s] [16:05:32] Start bump-pydantic.                                       main.py:61
[   17s]            Found 21 files to process.                                 main.py:78
[   20s] 
[   26s] 
[   26s] --- tt_tools_common/reset_common/host_reset_log.py
[   26s] +++ tt_tools_common/reset_common/host_reset_log.py
[   26s] @@ -153,7 +153,7 @@
[   26s]      """Model for storing information about a mobo reset"""
[   26s]  
[   26s]      #  allow for either an int or None
[   26s] -    nb_host_pci_idx: Union[List, None]
[   26s] +    nb_host_pci_idx: Union[List, None] = None
[   26s]      mobo: str
[   26s]      credo: List
[   26s]      disabled_ports: List
[   26s] [16:05:41] Run successfully!                                         main.py:154
[   14s] + bump-pydantic tt_smi
[   15s] [15:11:24] Start bump-pydantic.                                       main.py:61
[   15s]            Found 6 files to process.                                  main.py:78
[   17s] 
[   23s] 
[   23s] [15:11:32] No files were modified.                                   main.py:146
[   23s]            Run successfully!                                         main.py:154
warthog9 commented 1 month ago

What distro is this?

afaerber commented 1 month ago

openSUSE Tumbleweed, a rolling release.

warthog9 commented 1 month ago

Ok let me get my test box upgraded from F39 to F40, which should put me in roughly the same version range as Tumbleweed and see what's going on there.

warthog9 commented 1 month ago

Ok confirmed, and I've got a patch I'll push for testing by @sbansalTT in a minute. Don't think we are quite prepared to move to pydantic 2+ exclusively mostly because the base expected OS is Ubuntu 20.04, which has a MUCH older pydantic in it, and I'm hoping to get the rpm/deb packaging sorted (already have some RPMs, fighting with the debs right now) and leaving things on distro level packages where we can.

warthog9 commented 1 month ago

This should be sorted with the two commits @afaerber can you give it a go, we aren't seeing any issues on our test setups but would be good to have confirmation it's working on yours. Going to close this in the meantime, if it breaks re-open and we'll get it sorted.

afaerber commented 1 month ago

Confirming both patches apply cleanly on top of the tags and work. Thanks!