osparc-simcore icon indicating copy to clipboard operation
osparc-simcore copied to clipboard

Refactor `simcore_service_catalog.clients.director.DirectorApi.get_service_extras`

Open pcrespov opened this issue 1 year ago • 1 comments

Based on the iteration of PR https://github.com/ITISFoundation/osparc-simcore/pull/7491, please consider the refactoring simcore_service_catalog.clients.director.DirectorApi.get_service_extras

  • Should return a domain model (ServiceExtras is a schema model from directorv2?!)
  • Consider moving some of the logic wrapping the calls to the director request into the service layer i.e. simcore_service_catalog.services.service.get_service_extras. This way DirectorApi can remain a thick http client and some of the logic is moved to the service layer
  • The clients should never be used directly in the controllers
  • Please add tests at the service layer. NOTE that there is a respx fake of the director server mocked_director_service_api

@GitHK Since you are the author of that logic, i let you take the lead. I am happy to assist if you have questions.

pcrespov avatar Apr 10 '25 13:04 pcrespov

Splitting the logic between the client and the service layer will reduce as well the cognitive complexity of the current implementation. Some of the concepts in the current implementation are mostly resposibility of the service layer than the client whose main responsibility should be communicate reliably with the external service

Image

pcrespov avatar Apr 10 '25 17:04 pcrespov