Dynamic pagination for _get_groups and _get_objects

[RasmusThing]

No description provided.

[RasmusThing]

Fixes #217 and #218.

[falkowich]

I'm working thru the tests that went obsolete with this PR.
Getting to you as soon as this is done..

--
Kind Regards Falk

[falkowich]

I'm considering the continued use of the 'size' parameter, setting a sensible default limit, such as 100 rows per request. If we have a system with thousands of endpoints. I think it would be nice to have a default behavior that doesn't attempt to retrieve all data in a single, potentially overwhelming request.

By retaining the 'size' parameter, we're able to impose an upper limit on the number of entries returned per request by default, thereby preventing potential server overload and latency.

Any thougts?

--
Kind Regards

[RasmusThing]

Sure. I simply set the limit to the maximum size allowed by ISE (100), to avoid making too many unnecessary requests when pulling all objects.

If size should be retained, then page should as well, no? To allow the user to paginate themselves.

[falkowich]

Sure. I simply set the limit to the maximum size allowed by ISE (100), to avoid making too many unnecessary requests when pulling all objects.

If size should be retained, then page should as well, no? To allow the user to paginate themselves.

Perhaps an option for "get all / dynamic pagtionation" or something like that?
Otherwise as you say set the default limit to 100 and still use pagination, then we don't break backward compability to much?

Thougts?

--
Kind Regards Falk

[RasmusThing]

When using this module, I would expect the endpoints to return all entries.
It's not ideal for the end-user of the module, to have to create a pagination function themselves to get all entries from this module.

I'd say, if anything, an optional limit/page parameter and the default behavior is to get all entries.

[falkowich]

When using this module, I would expect the endpoints to return all entries. It's not ideal for the end-user of the module, to have to create a pagination function themselves to get all entries from this module.

True, indeed.

I'd say, if anything, an optional limit/page parameter and the default behavior is to get all entries.

Yes, but we "must" take care of backwards compability, or make some good informational communication that we change the default.
That's why I suggested "dynamic as optional". But perhaps that's just backward thinking :)

The dynamic pagination is great feature, and I too really want it to merge..

Any thoughts about my "mumbling"? :)

--
Kind Regards Falk

[falkowich]

I'll try to push a +10k endpoints into my dev env today and see how it reacts when doing a get with no "wait time" between the api requests.

In prod we have ~129k "total endpoints" and ~10k "active endpoints".
So I guess that this is the amount that will getting back if I try to do a get_endpoints with "no groupID".

[RasmusThing]

Sounds good.

Backwards compatibility can be done by still having the size and page parameter in the functions?
If not, sometimes you have to make a breaking change release :)

[falkowich]

Sounds good.

Backwards compatibility can be done by still having the size and page parameter in the functions? If not, sometimes you have to make a breaking change release :)

I agree, breaking changes is a must.
But it feels better to make it a deprecated function for some time before doing the breakage :)

Is it possible for you to make one PR for get_device_groups and dynamic pagination.
I can off-course split those, but then the PR will not be linked to you. :)

--
MvH Falk

[RasmusThing]

I removed the filter from get_device_groups and added that change to a new PR.
Sorry about the delay :)

[falkowich]

Great!

Real Life is the important thing here imho :)
So no worries at all :)

--
Kind Regards Falk