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

Sol1james patch 1 #1

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Sol1james patch 1 #1

wants to merge 5 commits into from

Conversation

sol1james
Copy link
Owner

@sol1james sol1james commented Feb 18, 2022

Updating doco to reflect the need to encapsulate the result set in a list when calling the all() method of an endpoint.

I'm not sure if it needs more explanation than this so I'm erring on the side of brevity.

See disscussion in netbox-community#450

Updating doco to reflect the need to encapsulate the result set in a list
@zachmoody
Copy link

Hey, thanks for the PR! I'm not sure we'd want to make this the canonical example for .all()'s use though. The draw for converting .all() and .filter() to generators in the first place was to be a little more efficient with our IO operations. Directing user's to always cast to a list causes them to miss out on those. It's certainly the way to do it in some cases like yours where you find yourself iterating over the results more than once, but consider the case in which a user is looking for something in the results they're unable to filter on. With generators, if that result is found on the first page of a large result, they can exit the loop and save the subsequent queries to pull down the rest of the results.

I'm not saying we shouldn't mention it, like I pointed out in the issue, it's been brought up more than once since the change. I just don't know about it being the example. If that makes any sense.

@sol1james
Copy link
Owner Author

I'll write a more nuanced description then. If someone does use the all() method, what do they then need to do to start iterating over the collection from the beginning again? Do they need to re-instantiate the API object with pynetbox.api()?

It would also be helpful to get some clarification about the mechanics of all this. The all() method takes a parameter 'limit' - I assume that when you call all() it will fetch this number of records and presumably cache them? And not do any more requests to the server across the network until the cache is empty?

And if you convert it to a list it will fetch the entirety of the result set which will be stored in the list?

It seems like the main question the user is faced with is whether they want to reduce network usage. If they do they use all(). If they want the data stored locally they use list(all())

@zachmoody
Copy link

I'll write a more nuanced description then. If someone does use the all() method, what do they then need to do to start iterating over the collection from the beginning again? Do they need to re-instantiate the API object with pynetbox.api()?

Thanks, I appreciate the effort to clarify. To start over from the beginning you'd call .all() or .filter() again, but it's likely much more efficient to just save the results in a list.

It would also be helpful to get some clarification about the mechanics of all this. The all() method takes a parameter 'limit' - I assume that when you call all() it will fetch this number of records and presumably cache them? And not do any more requests to the server across the network until the cache is empty?

Close, but no caching. Limit is, in effect, passed on to the NetBox paginator. So each time we need to make a call for the next page it contains the same number of items specified by limit. For example, If you're iterating over the RecordSet and passed a limit of 50, when you reach the 50th item we'll make a call for the next 50 items and so on until you're done traversing the results.

And if you convert it to a list it will fetch the entirety of the result set which will be stored in the list?

Yes

It seems like the main question the user is faced with is whether they want to reduce network usage. If they do they use all(). If they want the data stored locally they use list(all())

More-or-less, though I'd always encourage using .filter() with some arguments unless you're dealing with an endpoint that only returns a few items.

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.

2 participants