feat: add a business logic layer

[aldbr]

Details explained in #348

• Split diracx-db into 2 sub-directory: business and data_access.

• Update: create a diracx-logic layer

  • diracx-routers becomes the presentation layer: manages user authN/Z, calls the business logic layer and converts the returned results/raised exceptions into HTTP-friendly responses for the end users.
  • diracx-logic: deals with the business logic by performing operations and calling the data access layer to get additional data and persist them.
  • diracx-db: abstracts sql/no sql complexities.
  • auth: adapt diracx-routers and move business logic to diracx-logic.
  • jobs: adapt diracx-routers and move business logic to diracx-logic.
    • remove DIRAC from diracx-db.jobs.db
    • add type hinting as much as possible + docstrings
  • make diracx-db more simple (atomic operations + business logic moved to diracx-logic)
  • extensions

• Enforce consistency using import-linter

• Update: enforce consistency by adding the right dependencies in the right package:

  • diracx-routers depends on fastapi, not dirac nor sqlalchemy/opensearchpy.
  • diracx-logic depends on dirac, not fastapi nor sqlalchemy/opensearchpy.
  • diracx-db depends on sqlalchemy/opensearchpy, not dirac nor fastapi.
  • There is one limitation though: we can't easily programmatically prevent a developer from using dbs from diracx.routers because they are injected through diracx.routers.dependencies, so we would need to be careful when reviewing the code.

• documentation about the 3-layer architecture within diracx

[chaen]

I thought we had agreed not to do it in general, and only on adhoc cases ?

[aldbr]

Oops, I thought we said we would add a business logic layer within diracx.db: #348 (comment)

I thought it would be better to be consistent and have all the business logic at the same place to have a single responsibility principle between the layers + follow a standard architectural pattern.

[aldbr]

• First of all, sorry this PR is actually massive and very hard to review.

• Deployment/docker and Integration Tests are not passing, I think they would pass once feat: add diracx-logic in services-base container-images#31 will be merged (should be safe to merge IIUC).

• I remove some methods that were unused in diracx-db (mostly because they had a bulk version, which should be preferred)

• I remove DevelopmentSettings from the /dirac-metadata route (I had a circular dependency issue, which brings me to question why it was here and I haven't found any reason). If you think it's important, then I can revert that change.

[aldbr]

• Deployment/docker is passing because services-base includes logic
• Integration Tests are still not passing because services has not been updated yet (would require to merge another PR to trigger the deployment of a new dev tag)

[chaen]

You can create the dev tag yourself by triggering the deployment :-) Even if it creates a new real tag, it does not really matter... :-)

[aldbr]

Alright, all is 🟢 now

[fstagni]

It is close to impossible to properly review this PR, so these are just few comments.

For a PR that mostly should be a lot of "move around" of code, I am surprised that there are more than 700 lines of code added

[fstagni]

Is this something used (e.g. by the tests) or something here as an example?

[aldbr]

I don't think it's used, let's try to remove it and see if it breaks

[fstagni]

I know you did not start this file, anyway: what is the rationale for having all (?) models in it?

[aldbr]

Good question actually, I think we should agree on the purpose of this file and it would deserve a docstring.
I added all the models that were (i) passed as inputs to the routers; (ii) returned as outputs by the routers, so the ones that could be shared between the clients (cli, api) and the services (routers, logic, db).

At some point we should probably make a models directory with multiple modules such as jobs, auth...

[fstagni]

The linked issue is not solved. It does not apply here anymore?

[aldbr]

The linked issue is not solved:

• I break down the method into smaller methods in diracx-db to move the logic from it to diracx-logic.
• The comment was moved to diracx-logic because it makes more sense to read it from here now.

[fstagni]

Why remove this test?

[aldbr]

Because:

• I break down get_authorization_flow into smaller methods to move the logic from diracx-db to diracx-logic
• The new get_authorization_flow is now very simple and does not require complex testing as it was the case before: complex testing should now be done in diracx-routers and it's already done.