Skip to content

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

Merged
lgarber-akamai merged 4 commits into
linode:devfrom
lgarber-akamai:fix/mappedobject-dict-load-unpopulated
Apr 18, 2024
Merged

fix: Always load object values when converting MappedObjects to dict#400
lgarber-akamai merged 4 commits into
linode:devfrom
lgarber-akamai:fix/mappedobject-dict-load-unpopulated

Conversation

@lgarber-akamai

@lgarber-akamai lgarber-akamai commented Apr 18, 2024

Copy link
Copy Markdown
Contributor

📝 Description

This change resolves an issue that would cause unloaded objects to be serialized as None when 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_disk test with some minor changes in Ansible and everything passes as expected.

Blocks linode/ansible_linode#495

✔️ How to Test

Unit Testing

make testunit

Manual Testing

  1. In a linode_api4 sandbox environment (e.g. dx-devenv), run the following:
import os

from linode_api4 import LinodeClient, EventPoller

client = LinodeClient(os.getenv("LINODE_TOKEN"))

poller = EventPoller(
    client,
    entity_type="linode",
    action="linode_create",
)

inst = client.linode.instance_create(
    "g6-nanode-1",
    "us-mia",
    image="linode/alpine3.19",
    root_pass="c00lr00tp4ssw0rd!!!!!1!!!"
)

poller.set_entity_id(inst.id)
poller.wait_for_next_event_finished()

print(inst.configs[0].devices.dict["sda"])
  1. Ensure the output looks similar to the following:
{'id': 113268590, 'status': 'ready', 'label': 'Alpine 3.19 Disk', 'created': '2024-04-18T17:11:55', 'updated': '2024-04-18T17:12:12', 'filesystem': 'ext4', 'size': 25088}

This confirms the devices have been explicitly refreshed during the dict conversion.

@lgarber-akamai lgarber-akamai added the bugfix for any bug fixes in the changelog. label Apr 18, 2024
@lgarber-akamai
lgarber-akamai marked this pull request as ready for review April 18, 2024 17:12
@lgarber-akamai
lgarber-akamai requested a review from a team as a code owner April 18, 2024 17:12
@lgarber-akamai
lgarber-akamai requested review from jriddle-linode and yec-akamai and removed request for a team April 18, 2024 17:12
Comment on lines +162 to +163
if v.identifier:
continue

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

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

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 🙂

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

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

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

@ykim-akamai ykim-akamai left a comment

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.

LGTM, verified manual steps locally

@yec-akamai yec-akamai left a comment

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.

Nice fix! All tests passed on my end 🎉

if obj is None:
return None

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

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

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 🙂

@lgarber-akamai
lgarber-akamai merged commit 6ba2d10 into linode:dev Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix for any bug fixes in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants