Refactor library folder API logic into FoldersService #11631
Conversation
davelopez
added
the
kind/refactoring
lib/galaxy/managers/folders.py
Outdated
| @@ -309,3 +312,257 @@ def cut_and_decode(self, trans, encoded_folder_id): | |||
| :rtype: int | |||
| """ | |||
| return self.decode_folder_id(trans, self.cut_the_prefix(encoded_folder_id)) | |||
|
|
|||
|
|
|||
| class FoldersManager: | |||
There was a problem hiding this comment.
What is the difference between the FolderManager and FoldersManager? Should these be the same class?
There was a problem hiding this comment.
The s character... just kidding 😆
The idea is that FoldersManager encapsulates the logic for the FoldersController and uses FolderManager and RoleManager to implement it. This class will also use and return the future pydantic models.
But yeah... I agree the name is not the best... 😅
Do you think these two should go together?
There was a problem hiding this comment.
@jmchilton
Do you prefer this approach to deal with the class encapsulating the controller logic instead of calling it a Manager? Or am I going completely offroad here? 😆
Appreciated any advice on how to proceed with this :)
There was a problem hiding this comment.
I have renamed the FoldersManager to FoldersService to better differentiate the class encapsulating the API actions logic from the regular Managers used to deal with the database models.
I have also used this naming in #11712 to unify it a bit. Again, the idea is to have this layer that directly speaks "pydantic" and uses the appropriate managers under the hood to expose the API actions that can be then called from the API controllers (either legacy or FastAPI) in a one-liner to reuse as much logic as possible and keed the API controllers thin.
But I am really open to better naming ideas or changes in this approach :)
davelopez
force-pushed
the
refactor_library_folder_api
branch
from
71fed5a
to
ce8f8b9
Compare
This is to better differentiate the class encapsulating the API actions logic from the regular Manager used to deal with the database models.
davelopez
force-pushed
the
refactor_library_folder_api
branch
from
ce8f8b9
to
3919fc5
Compare
davelopez
changed the title
|
Awesome - thanks so much! I'm fine with the new naming convention of managers vs services - makes a lot of sense. |
What did you do?
UsesLibraryMixin,UsesLibraryMixinItems) fromFoldersController.FoldersControllerlogic into theFoldersManagerclass to be reused by the future FastAPI controller.Why did you make this change?
This will simplify the migration to FastAPI of this route in a future PR as well as provide some automated tests as a harness.
How to test the changes?