Skip to content
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

Add custom model name lookup to fix #389 #390

Merged
merged 8 commits into from
Dec 4, 2021

Conversation

markkuleinio
Copy link
Contributor

@markkuleinio markkuleinio commented Jun 26, 2021

Fixes #389

The list_parser() function in Record._parse_values() is modified to support lists of custom models, defined like this:

class Permissions(Record):
    users = [Users]

Running example:

>>> perm = list(nb.users.permissions.all())[0]
>>> pprint(dict(perm))
{'actions': ['view'],
 'constraints': {'status': 'test'},
 'description': '',
 'display': 'TestPermission',
 'enabled': False,
 'groups': [],
 'id': 1,
 'name': 'TestPermission',
 'object_types': ['circuits.circuit'],
 'url': 'http://netbox31beta.lein.io/api/users/permissions/1/',
 'users': [{'display': 'admin',
            'id': 1,
            'url': 'http://netbox31beta.lein.io/api/users/users/1/',
            'username': 'admin'}]}
>>> perm.users
[admin]
>>> type(perm.users[0])
<class 'pynetbox.models.users.Users'>
>>>

The original PR text left here for the memory of all the unsuccessful attempts: 😁

The model name lookup table is populated in the model source file (users.py) to prevent circular imports (we cannot import Users in response.py because users.py already imports response.py).

Works now:

>>> permission = list(netbox.users.permissions.all())[0]
>>> permission.users
[test1]
>>>

@Dimaqa
Copy link
Contributor

Dimaqa commented Sep 28, 2021

I believe the idea was to pass return_obj to Endpoint class and implement full Permission model not only users field.
So after you define

class User:
     def __str__(self):
        return self.username

class Permission:
     users = User

     @property
     def some_usefull_property(self):
          return 42

and somehow pass Permission model to permissions Endpoint you get the same behaviour as with default defined models.
This implementation would also be really helpful with plugins integration.

@Dimaqa
Copy link
Contributor

Dimaqa commented Sep 28, 2021

I think you just need to find a way to pass your new model to App class or modify _lookup_ret_obj in Endpoint class

@markkuleinio
Copy link
Contributor Author

Any new comments anyone? @Dimaqa @zachmoody

Is it a problem that Permission.users is a list of User objects, not just one User?

Lost track of all these classes and models already 😄 but thinking that something needs to be figured out to support modern NetBox versions. Is it then dealing with all kinds of custom models, or fixing the __str__() output with the display field as a last resort in

return getattr(self, "name", None) or getattr(self, "label", None) or ""
(because that's the common and good enough (for me, I think now, but untested) solution for NetBox 2.11+ objects).

@Dimaqa feel free to open your own PR if you have a better fix in mind.

@markkuleinio
Copy link
Contributor Author

Not sure how @zachmoody gets the notifications for a new commit in an existing PR so here is a ping for you 😁 (see the modified message on top of this PR)

@zachmoody zachmoody merged commit c702fd1 into netbox-community:master Dec 4, 2021
@markkuleinio markkuleinio deleted the field_name_lookup branch December 5, 2021 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

users in permissions is represented as empty strings
3 participants