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

Move database commits from service layer to controller layer #257

Closed
1 task
jkglasbrenner opened this issue Aug 8, 2023 · 2 comments
Closed
1 task

Move database commits from service layer to controller layer #257

jkglasbrenner opened this issue Aug 8, 2023 · 2 comments
Labels
refactor Changes that neither fix a bug nor add a feature wontfix This will not be worked on

Comments

@jkglasbrenner
Copy link
Collaborator

jkglasbrenner commented Aug 8, 2023

Blocked by #312 #313 #319

Definition of Done

  • All calls to db.session.commit() are moved out of methods in respective service.py files and invoked at the bottom of each method in controller.py.
@jkglasbrenner jkglasbrenner added this to the Backlog milestone Aug 8, 2023
@jkglasbrenner jkglasbrenner added the refactor Changes that neither fix a bug nor add a feature label Aug 8, 2023
@hbooth hbooth removed this from the Backlog milestone Oct 10, 2023
@keithmanville keithmanville added the blocked Unable to move forward because a dependency has yet to be completed label Nov 7, 2023
@alexb1200
Copy link
Contributor

Based on the current REST api refactors this may no longer be relevant. @jkglasbrenner suggested some questions to determine what our next steps should be.

  1. Where are commits happening for each major endpoint?
  2. Are they happening earlier than necessary? Basically, a commit is needed if another function needs to use the object's ID in some way, which most of the time will happen during a POST request (the database autogenerates it and returns it upon commit).
  3. Does the API currently allow multiple changes to occur across database tables in a single request? If so, are we issuing multiple commits?

After looking through each endpoint:

  1. For each endpoint commits are happening at the end of calls to create or update functions. This is usually the last function called by the controller layer before returning the response. This is true for every endpoint.
  2. A commit is only occurring when the object is changed or created i.e. a put or post, and based on the way the controller is calling functions from the service layer the commit is one of the last actions to occur. Because just about every other endpoint will use the object's id in following calls to the REST api I don't think we could call it any later.
  3. No, in each endpoint only one change occurs per call to the api. There is only one call to commit per request for each endpoint. The most complex use of the service layer is in PUT requests which sometimes use a get call to the service layer to turn a name reference into an id reference before changing the object (the only place the commit occurs) and then returning the response.

Based on this, I don't think there's any need for an immediate change.

@jkglasbrenner
Copy link
Collaborator Author

This will be resolved in the v1 release through the creation of a dedicated data access layer and keeping all behavior encapsulated in the service layer, with the controller calling no more than one service method per endpoint. The v1 ORM is set up so that most, if not all, endpoint operations can be handled in a single transaction.

@jkglasbrenner jkglasbrenner added wontfix This will not be worked on and removed blocked Unable to move forward because a dependency has yet to be completed labels May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Changes that neither fix a bug nor add a feature wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants