Skip to content

fix: Always load object values when converting MappedObjects to dict #400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgarber-akamai merged 4 commits into from
Apr 18, 2024
Merged

fix: Always load object values when converting MappedObjects to dict #400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2171f77
Always load object values when converting MappedObjects to dict
lgarber-akamai Apr 18, 2024
18ef93e
Prevent overriding identifier attributes during construction
lgarber-akamai Apr 18, 2024
6a01614
make format
lgarber-akamai Apr 18, 2024
89c3dfd
Update unit test to cover parent_id issue
lgarber-akamai Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions linode_api4/objects/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import time
from datetime import datetime, timedelta
from typing import Any, Dict, Optional

from linode_api4.objects.serializable import JSONObject

Expand Down Expand Up @@ -99,6 +100,18 @@ def _expand_vals(self, target, **vals):
def __repr__(self):
return "Mapping containing {}".format(vars(self).keys())

@staticmethod
def _flatten_base_subclass(obj: "Base") -> Optional[Dict[str, Any]]:
if obj is None:
return None

# If the object hasn't already been lazy-loaded,

@yec-akamai yec-akamai Apr 18, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, in which cases that a object isn't lazy-loaded?

@lgarber-akamai lgarber-akamai Apr 18, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 馃檪

# manually refresh it
if not getattr(obj, "_populated", False):
obj._api_get()

return obj._raw_json

@property
def dict(self):
result = vars(self).copy()
Expand All @@ -112,12 +125,17 @@ def dict(self):
(
item.dict
if isinstance(item, cls)
else item._raw_json if isinstance(item, Base) else item
else (
self._flatten_base_subclass(item)
if isinstance(item, Base)
else item
)
)
for item in v
]
elif isinstance(v, Base):
result[k] = v._raw_json
result[k] = self._flatten_base_subclass(v)

return result


Expand All @@ -140,7 +158,10 @@ def __init__(self, client: object, id: object, json: object = {}) -> object:
#: be updated on access.
self._set("_raw_json", None)

for k in type(self).properties:
for k, v in type(self).properties.items():
if v.identifier:
continue
Comment on lines +162 to +163

@lgarber-akamai lgarber-akamai Apr 18, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make sure we don't override identifier attributes


self._set(k, None)

self._set("id", id)
Expand Down
4 changes: 2 additions & 2 deletions linode_api4/objects/dbase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ class DerivedBase(Base):
parent_id_name = "parent_id" # override in child classes

def __init__(self, client, id, parent_id, json={}):
Base.__init__(self, client, id, json=json)

self._set(type(self).parent_id_name, parent_id)

@lgarber-akamai lgarber-akamai Apr 18, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needed to be repositioned so the _populate called from the Base constructor has access to the parent ID


Base.__init__(self, client, id, json=json)

@classmethod
def _api_get_derived(cls, parent, client):
base_url = "{}/{}".format(
Expand Down
4 changes: 2 additions & 2 deletions linode_api4/objects/object_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ class ObjectStorageBucket(DerivedBase):
id_attribute = "label"

properties = {
"cluster": Property(),
"cluster": Property(identifier=True),
"created": Property(is_datetime=True),
"hostname": Property(),
"label": Property(),
"label": Property(identifier=True),
Comment on lines +34 to +37

@lgarber-akamai lgarber-akamai Apr 18, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These weren't flagged as IDs so they were always overridden after the above change. This should fix that up 馃檪

"objects": Property(),
"size": Property(),
}
Expand Down
8 changes: 8 additions & 0 deletions test/unit/objects/linode_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,14 @@ def test_interface_ipv4(self):
self.assertEqual(ipv4.vpc, "10.0.0.1")
self.assertEqual(ipv4.nat_1_1, "any")

def test_config_devices_unwrap(self):
"""
Tests that config devices can be successfully converted to a dict.
"""

inst = Instance(self.client, 123)
assert inst.configs[0].devices.dict.get("sda").get("id") == 12345


class StackScriptTest(ClientBaseCase):
"""
Expand Down