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

CASMCMS-9225: Create generic endpoint classes #397

Open
wants to merge 1 commit into
base: casmcms-9225-full
Choose a base branch
from

Conversation

mharding-hpe
Copy link
Contributor

CASMCMS-9225 as a whole involves changes to a number of different files in BOS. In order to aid with review, I'm breaking the overall thing up into smaller PRs. Each will be built on top of each other, and will be merging into the casmcms-9225-full branch. Only once each sub-PR has been approved and merged will I merge that branch into develop.

When investigating the apparent resource leaks in BOS, I discovered that simply preventing the request timeouts or limiting the response sizes was not sufficient. I found that I also needed to place context managers around request sessions, HTTP adapters, and request responses. (Note -- I also learned that only using the context managers was not sufficient to solve the problem). The problem is, the BOS code uses requests sessions in various places. In some cases, the session is passed in as an optional argument. In other cases, there are long-running sessions, or sessions that last the lifetime of the BOS operators. I realized that to avoid making a ton of hacky fixes all over the place, it would simplify things if I refactored the code BOS uses when interacting with APIs.

This PR is the first step in that process. It introduces classes that represent API endpoints.

  • BaseGenericEndpoint is the fundamental endpoint class. It defines some of the common functions (like how to construct the endpoint URLs from a URI, and how to make the API requests). It does not make assumptions about what data the caller wants back from the request. That is why it is a Generic. It provides a default exception handling strategy, but it can be overridden in subclasses. I did this because this was another case where we had a fairly scattershot approach in the repo to how we dealt with request exceptions. A couple of places did basically the same thing, but repeated largely the same code. Other places did not handle all of the possibly exceptions.
  • BaseRequestErrorHandler is the fundamental exception handler class. It could also have been defined as a protocol, since it is essentially just specifying what an error handler must do in order to work with the endpoint class.
  • RequestErrorHandler is the default exception handler class. I created it based on the best exception handling code that we had in the BOS repo. It is created in such a way that it can be easily subclassed if all you want to do it handle one specific exception type differently (this was the case with IMS, as will be seen in a later PR).
  • BaseEndpoint is the "default" base endpoint class. By default I mean that it is the one most of the endpoints use, because it just returns the content of the body of the response (after parsing it as JSON, if it is not None). That is what most of the BOS API clients care about.
  • BaseRawEndpoint is the other endpoint class. It returns a collection of data from the response, in the form of the ResponseData class. This is used so that we can put the actual request response inside a context manager, and close it out. The BSS client endpoint ends up needing data from the header of the response, so it ends up using this class.

This PR does NOT make use of any of the above classes yet. The following PRs will do that.

@mharding-hpe mharding-hpe force-pushed the casmcms-9225-02-endpoints branch 3 times, most recently from 90aa877 to 725d4a6 Compare December 17, 2024 21:17
Base automatically changed from casmcms-9225-01-paging to casmcms-9225-full December 17, 2024 23:19
@mharding-hpe mharding-hpe force-pushed the casmcms-9225-02-endpoints branch from 725d4a6 to fda1560 Compare December 17, 2024 23:23
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