fix: Always load object values when converting MappedObjects to dict - #400
Conversation
| if v.identifier: | ||
| continue |
There was a problem hiding this comment.
We need to make sure we don't override identifier attributes
| "cluster": Property(identifier=True), | ||
| "created": Property(is_datetime=True), | ||
| "hostname": Property(), | ||
| "label": Property(), | ||
| "label": Property(identifier=True), |
There was a problem hiding this comment.
These weren't flagged as IDs so they were always overridden after the above change. This should fix that up 🙂
| def __init__(self, client, id, parent_id, json={}): | ||
| Base.__init__(self, client, id, json=json) | ||
|
|
||
| self._set(type(self).parent_id_name, parent_id) |
There was a problem hiding this comment.
This needed to be repositioned so the _populate called from the Base constructor has access to the parent ID
ykim-akamai
left a comment
There was a problem hiding this comment.
LGTM, verified manual steps locally
yec-akamai
left a comment
There was a problem hiding this comment.
Nice fix! All tests passed on my end 🎉
| if obj is None: | ||
| return None | ||
|
|
||
| # If the object hasn't already been lazy-loaded, |
There was a problem hiding this comment.
Just curious, in which cases that a object isn't lazy-loaded?
There was a problem hiding this comment.
The most notable example would be the underlying Disk/Volume objects for an Instance config's device mapping. They're not loaded when their objects are created here to save on unnecessary API requests and are explicitly lazy-loaded when one of their attributes is accessed.
Since we access _raw_json directly, we need to forcibly load them since they won't be automatically lazy-loaded 🙂
📝 Description
This change resolves an issue that would cause unloaded objects to be serialized as
Nonewhen converting a MappedObject to a dict. This works by explicitly refreshing the object if it has not already been populated.Additionally, this change fixes a small bug that prevented parent IDs from being accessed in
_populate(...)overrides.This has been cross-tested against the Ansible
instance_config_disktest with some minor changes in Ansible and everything passes as expected.Blocks linode/ansible_linode#495
✔️ How to Test
Unit Testing
Manual Testing
This confirms the devices have been explicitly refreshed during the dict conversion.