Skip to content

Unexpected re-hydration on isinstance-check #688

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.

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

Already on GitHub? Sign in to your account

Open
mimukr opened this issue May 20, 2025 · 0 comments
Open

Unexpected re-hydration on isinstance-check #688

mimukr opened this issue May 20, 2025 · 0 comments
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@mimukr
Copy link

mimukr commented May 20, 2025

pynetbox version

v.7.4.1

NetBox version

v4.2.6

Python version

3.11

Steps to Reproduce

When doing isinstance-checks on a Record, there's a chance of re-hydration happening, with two unwanted effects:

  1. An unnecessary network call is made
  2. Any updates done on the Record is reset

For instance, the internal instance check of BaseModel of Pydantic, will perform a hasattr(instance, '__pydantic_decorators__') on the Record, triggering Record.__getattr__ and thus a re-hydration.

Example:

import pynetbox
from pydantic import BaseModel

nb = pynetbox.api(...)

device = nb.dcim.devices.get(1) # Any object works

device.name = "New Name"

isinstance(device, BaseModel) # Will trigger network call and re-hydration/resetting of values

assert device.name != "New Name" # Local name update will have been reset

Expected Behavior

Local updates on the Record should persist after __getattr__ is called by overloaded instance checks. Furthermore, not doing a network call in this case would be preferable.

Specifically, I'm suggesting the following change to Record.__getattr__.

def __getattr__(self, k):
    if self.url:
        if self.has_details is False and k != "keys" and not k.startswith("__"): # <---- Fix 1: No need for network calls for dunder attrs
            if self.full_details():
                ret = getattr(self, k, None)
                if ret or hasattr(self, k):
                    return ret

    raise AttributeError('object has no attribute "{}"'.format(k))

along with the following change to Record.full_details

def full_details(self):
    if self.url:
        req = Request(
            base=self.url,
            token=self.api.token,
            http_session=self.api.http_session,
        )
        curr_updates = self.updates()        # <---- Fix 2: Save current updates
        self._parse_values(next(req.get()))
        for k, v in curr_updates.items():    # <---- Fix 2: Restore
            setattr(self, k, v)              # <---- current updates
        self.has_details = True
        return True
    return False

Observed Behavior

See code comments in "Steps to Reproduce"

@mimukr mimukr added type: bug A confirmed report of unexpected behavior in the application status: needs triage The issue is awaiting triage by a maintainer labels May 20, 2025
@bctiemann bctiemann added severity: low Does not significantly disrupt application functionality, or a workaround is available status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: needs triage The issue is awaiting triage by a maintainer labels May 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

2 participants