canonical / jhack

Chock-full of Juju hackery.
Apache License 2.0
50 stars 24 forks source link

fix(recorder): Python 3.11 dataclass default factory for mutable objects #116

Closed thp-canonical closed 9 months ago

thp-canonical commented 9 months ago

Running jhack on Ubuntu 23.10 using Python 3.11 results in the following error:

Traceback (most recent call last):
  File "/home/thp/src/jhack/./venv/bin/jhack", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/thp/src/jhack/venv/lib/python3.11/site-packages/jhack/main.py", line 40, in main
    from jhack.utils.event_recorder.client import (
  File "/home/thp/src/jhack/venv/lib/python3.11/site-packages/jhack/utils/event_recorder/client.py", line 18, in <module>
    from jhack.utils.event_recorder.recorder import DEFAULT_DB_NAME, Event, event_db
  File "/home/thp/src/jhack/venv/lib/python3.11/site-packages/jhack/utils/event_recorder/recorder.py", line 499, in <module>
    @dataclass
     ^^^^^^^^^
  File "/usr/lib/python3.11/dataclasses.py", line 1230, in dataclass
    return wrap(cls)
           ^^^^^^^^^
  File "/usr/lib/python3.11/dataclasses.py", line 1220, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/dataclasses.py", line 958, in _process_class
    cls_fields.append(_get_field(cls, name, type, kw_only))
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/dataclasses.py", line 815, in _get_field
    raise ValueError(f'mutable default {type(f.default)} for field '
ValueError: mutable default <class 'jhack.utils.event_recorder.recorder.Context'> for field context is not allowed: use default_factory

This is caused by a change in Python 3.11, from the Mutable default values docs:

Changed in version 3.11: Instead of looking for and disallowing objects of type list, dict, or set, unhashable objects are now not allowed as default values. Unhashability is used to approximate mutability.

This change does change the behavior - previously the context field of Scene instances has been - accidentally? - shared between all instances (that don't overwrite the default value), whereas this changes makes it so that each instance of Scene has its own Context.

This is not an issue for Scene.from_dict() (because context is explicitly initialized there), but might be an issue in other instantiations of Scene that don't initialize context (e.g. in _record_current_event()) and expect the context attribute to be shared(?).