pynautobot icon indicating copy to clipboard operation
pynautobot copied to clipboard

Housekeeping: Refactor all duplicated calls to `Request` to a base class with a method

Open jathanism opened this issue 2 years ago • 2 comments

Proposal

I propose that a serious refactor is performed to reduce all of the code duplication in this library predominantly around how Request objects are repeatedly constructed and the code to do so is duplicated all over the library.

This was exposed when we recently began the work on adding support for API versioning by adding the api_version= argument, which requires updating the calls in numerous places when it should be done in ONE and only one place.

Justfication

For example, consider the source for pynautobot.core.api where the Api class has three properties: Api.version vs. Api.openapi vs Api.status. Each of these is making the exact same duplicated call to Request(base=self.base_url, http_session=self.http_session, api_version=self.api_version) and then calling a method on the Request object (get_version(), get_openapi(), and get_status() respectively):

https://github.com/nautobot/pynautobot/blob/3956bd928c36eb87be0048971d02e61e2743a44b/pynautobot/core/api.py#L91-L165

Following the code path to the source for pynautobot.core.query.Request, you can then see that each of these methods ALSO has nearly the exact same code duplicated for each instead of calling a centralized method that already sets the same header value and input parameters:

https://github.com/nautobot/pynautobot/blob/3956bd928c36eb87be0048971d02e61e2743a44b/pynautobot/core/query.py#L145-L198

Longer term this is not a maintainable pattern. There are so many individual calls to create Request instances where instead each of the various Api and Endpoint objects or anything that is behaving as a client to the API should inherit from a common base and call the same underlying method on every request using the same centralized code path.

This will make the code easier to maintain and easier to use across the board and assert that when we want to extend or augment existing functionality to support new API features, they can be done in a single place, and won't require scouring the entire source tree.

jathanism avatar Apr 11 '22 16:04 jathanism

In this refactor, should we also look at moving from using Requests to perhaps HTTPX that may get some async capabilities for users while maintaining the similar Requests API calls?

jvanderaa avatar Apr 11 '22 16:04 jvanderaa

HTTPX

Worth considering as part of the scoping at a minimum. First I've personally heard of httpx.

jathanism avatar Apr 12 '22 17:04 jathanism