You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fix follow-up to #2185 / commit 4c28494. Several Celery tasks in osism.tasks.netbox and osism.tasks.openstack return raw pynetbox Record/RecordSet objects or OpenStack SDK Resource objects. These types are not serializable by Celery's default JSON result backend. Calling any of them via .delay() / .apply_async() / .si() surfaces the same Object of type <X> is not JSON serializable failure that we already saw for osism sonic list.
The tasks currently do not fail only because every existing caller invokes them as plain in-process function calls (no wire transport, no serialization). Any future migration to async dispatch — for example from the FastAPI layer, the listener, or a new CLI command — will trigger the bug.
This issue tracks converting these tasks to return plain JSON-serializable dicts/lists, so they are safe to dispatch async, matching the pattern used by conductor.get_sonic_devices after commit 4c28494.
Motivation
Prevent regressions: the current structural issue is a latent bug waiting to surface the next time someone writes netbox.get_device_by_name.delay(...) from an API handler.
Consistency: after 4c28494 the SONiC listing task returns serialization-safe dicts, but peer tasks in the same package still return raw ORM-like objects. This is confusing for new contributors.
API readiness: osism.api is adding more endpoints that delegate to Celery workers. Returning plain dicts from those tasks is a prerequisite.
baremetal_node_set_boot_device (line 356) and baremetal_port_delete (line 398) already return None or a delete-acknowledgement and are likely fine — double-check during implementation.
Callers that need to be updated
Grep results showing in-process consumers that currently rely on the raw object:
osism/tasks/conductor/ironic.py (many call sites around lines 360–812) — uses .uuid, .id, ["uuid"], .role.slug, etc.
Each caller must switch from attribute access on the raw object to dict-key access on the serialized shape.
Proposed approach (Variant 1)
Mirror the pattern introduced in commit 4c28494 for conductor.get_sonic_devices:
Serialization helpers. Add small helpers (e.g. _serialize_device, _serialize_interface, _serialize_ip_address in osism/tasks/netbox.py; _serialize_node, _serialize_port, _serialize_image, _serialize_network in osism/tasks/openstack.py). Each helper:
accesses fields defensively with getattr(..., default) / .get(...) so a single malformed record does not break the whole list,
returns only primitive types (str, int, bool, None, list, dict),
includes all fields today's callers rely on (see caller audit above).
Task bodies. Replace the raw return with return _serialize_X(result) (or a list comprehension for *_list / filter tasks).
Error signalling. Adopt the same convention as get_sonic_devices: raise RuntimeError with a descriptive message for error conditions (not found, wrong role, backend failure) instead of returning None/sentinel values. Empty results return [] (lists) or None (single-object lookups, only when ignore_missing=True).
Drop bind=True where self is unused. Several of these tasks declare self only to satisfy the decorator contract. If no self.request.* access is needed, remove bind=True and the self parameter. (This mirrors the get_devices cleanup in 4c28494 where the stray self was silently swallowing the positional argument.)
Update every caller. For each migrated task, switch in-process consumers from attribute access to dict-key access. Keep attribute-style field names in the serialized dict (e.g. uuid, provision_state, power_state) to minimize churn.
Tests. Add unit tests that import each task and assert the return value is JSON-serializable (json.dumps(result) must not raise). This guards against regressions and makes the contract explicit.
Non-goals
Changing the network topology of Celery queues / routing.
Switching result_serializer away from JSON (pickle would mask the real issue and introduce security risk).
Adding new functionality to any of these tasks. This is a targeted refactor.
Rewriting the callers to use async dispatch — they can stay synchronous, but they should not break when the task output becomes a dict.
Checklist
netbox.get_devices — serialize + update caller in conductor/ironic.py:758 and commands/baremetal.py:882
netbox.get_device_by_name — serialize (no external callers today; still fix the shape)
Summary
Fix follow-up to #2185 / commit 4c28494. Several Celery tasks in
osism.tasks.netboxandosism.tasks.openstackreturn raw pynetboxRecord/RecordSetobjects or OpenStack SDKResourceobjects. These types are not serializable by Celery's default JSON result backend. Calling any of them via.delay()/.apply_async()/.si()surfaces the sameObject of type <X> is not JSON serializablefailure that we already saw forosism sonic list.The tasks currently do not fail only because every existing caller invokes them as plain in-process function calls (no wire transport, no serialization). Any future migration to async dispatch — for example from the FastAPI layer, the listener, or a new CLI command — will trigger the bug.
This issue tracks converting these tasks to return plain JSON-serializable dicts/lists, so they are safe to dispatch async, matching the pattern used by
conductor.get_sonic_devicesafter commit 4c28494.Motivation
netbox.get_device_by_name.delay(...)from an API handler.osism.apiis adding more endpoints that delegate to Celery workers. Returning plain dicts from those tasks is a prerequisite.Scope
Affected tasks —
osism/tasks/netbox.pyget_devices(**query)nb.dcim.devices.filter(**query)(RecordSet)get_device_by_name(name)nb.dcim.devices.get(name=name)(Record or None)get_interfaces_by_device(device_name)nb.dcim.interfaces.filter(...)(RecordSet)get_addresses_by_device_and_interface(device_name, interface_name)nb.dcim.addresses.filter(...)(RecordSet)Affected tasks —
osism/tasks/openstack.pyimage_get(image_name)conn.image.find_image(...)(Resource)network_get(network_name)conn.network.find_network(...)(Resource)baremetal_node_create(node_name, attributes=None)baremetal_node_delete(node_or_id)baremetal_node_update(node_id_or_name, attributes=None)baremetal_node_show(node_id_or_name, ignore_missing=False)baremetal_node_list()list(conn.baremetal.nodes())(list of Resources)baremetal_node_validate(node_id_or_name)baremetal_node_wait_for_nodes_provision_state(node_id_or_name, state)baremetal_node_set_provision_state(node, state)baremetal_node_set_power_state(node, state, wait=False, timeout=None)baremetal_port_list(details=False, attributes=None)baremetal_port_create(attributes=None)Callers that need to be updated
Grep results showing in-process consumers that currently rely on the raw object:
osism/tasks/conductor/ironic.py(many call sites around lines 360–812) — uses.uuid,.id,["uuid"],.role.slug, etc.osism/tasks/conductor/config.py(lines 35, 51, 68, 82, 99) — usesresult.idon image/network results.osism/tasks/conductor/utils.py(line 157) — uses the returned Ironic node.osism/commands/baremetal.py(line 882) — usesnetbox.get_devices(...).Each caller must switch from attribute access on the raw object to dict-key access on the serialized shape.
Proposed approach (Variant 1)
Mirror the pattern introduced in commit 4c28494 for
conductor.get_sonic_devices:_serialize_device,_serialize_interface,_serialize_ip_addressinosism/tasks/netbox.py;_serialize_node,_serialize_port,_serialize_image,_serialize_networkinosism/tasks/openstack.py). Each helper:getattr(..., default)/.get(...)so a single malformed record does not break the whole list,str,int,bool,None,list,dict),returnwithreturn _serialize_X(result)(or a list comprehension for*_list/filtertasks).get_sonic_devices: raiseRuntimeErrorwith a descriptive message for error conditions (not found, wrong role, backend failure) instead of returningNone/sentinel values. Empty results return[](lists) orNone(single-object lookups, only whenignore_missing=True).bind=Truewhereselfis unused. Several of these tasks declareselfonly to satisfy the decorator contract. If noself.request.*access is needed, removebind=Trueand theselfparameter. (This mirrors theget_devicescleanup in 4c28494 where the strayselfwas silently swallowing the positional argument.)uuid,provision_state,power_state) to minimize churn.json.dumps(result)must not raise). This guards against regressions and makes the contract explicit.Non-goals
result_serializeraway from JSON (pickle would mask the real issue and introduce security risk).Checklist
netbox.get_devices— serialize + update caller inconductor/ironic.py:758andcommands/baremetal.py:882netbox.get_device_by_name— serialize (no external callers today; still fix the shape)netbox.get_interfaces_by_device— serialize + updateconductor/ironic.py:825netbox.get_addresses_by_device_and_interface— serializeopenstack.image_get— serialize + updateconductor/config.py:35,51,68openstack.network_get— serialize + updateconductor/config.py:82,99openstack.baremetal_node_create— serialize + updateconductor/ironic.py:370openstack.baremetal_node_delete— serialize + updateconductor/ironic.py:812openstack.baremetal_node_update— serialize + updateconductor/ironic.py:396,513,538,564openstack.baremetal_node_show— serialize + updateconductor/ironic.py:360,596,conductor/utils.py:157openstack.baremetal_node_list— serialize + updateconductor/ironic.py:773,1071openstack.baremetal_node_validate— serialize + updateconductor/ironic.py:436openstack.baremetal_node_wait_for_nodes_provision_state— serialize + update all callers inconductor/ironic.pyopenstack.baremetal_node_set_provision_state— serialize + update all callersopenstack.baremetal_node_set_power_state— serialize + updateconductor/ironic.py:461openstack.baremetal_port_list— serialize + updateconductor/ironic.py:406,656,808openstack.baremetal_port_create— serialize + updateconductor/ironic.py:422baremetal_node_set_boot_deviceandbaremetal_port_deletereturn JSON-safe valuesjson.dumps(task_result)succeeds for each taskbind=True+selfwhere unusedAcceptance criteria
json.dumps(task(...))succeeds for every task in a unit test (mocked NetBox / Ironic fixtures).osism baremetal *,osism netbox *,osism sync ironic).Reference
_serialize_deviceinosism/tasks/conductor/sonic/device.py.