Skip to content
This repository has been archived by the owner on Apr 9, 2023. It is now read-only.

Restructuring top-level main & init #78

Open
why-not-try-calmer opened this issue Sep 8, 2021 · 1 comment
Open

Restructuring top-level main & init #78

why-not-try-calmer opened this issue Sep 8, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@why-not-try-calmer
Copy link
Collaborator

why-not-try-calmer commented Sep 8, 2021

Currently we have:

├── defrag
│   ├── __init__.py  (1)
│   ├── __main__.py  (2)
│   ├── modules
│   │   ├── helpers  (3)
│   ├── profiling
│   └── tests

This comes with a number of caveats.

  • gunicorn requires the package.module:object syntax. This means we have to the run the application with something like gunicorn -w 4 -k uvicorn.workers.UvicornWorker defrag.main:app. __main__.py is not expressible in this syntax as far as I know.
  • as we close in to deployment the distinction between (1) and (2) is not clear. (1) does the job of enumerating enabled modules and exposing them to the entire package, while (2) does the job of registering modules-as-services and to expose special handlers ('startup', 'shutdown'). But the way this is written currently does not work if we run the app as mentioned in the previous bullet point, because the main function defined in (2) is not accessible to gunicorn, as gunicorn needs to latch on to the actually application object exposed by fastAPI. There is simple solution (tested here) but it's not the most beautiful one.
  • helpers (3) are considered modules, but there is no need to since they are pure functions and don't depend on any state for their functionality. Also, some portions of code across several modules depend on helpers. Making them modules suggest that nothing depends on them and/or that they are stateful, when neither is true.

Recommendations:

  • let's rename __main__.py to main.py
  • let's move all the business logic dealing with importing modules and registering modules as services to (1), and call the registration unconditionnally there; let's have (2) only expose he special handlers as it already does.
  • let's move (3) directly under defrag/
@why-not-try-calmer why-not-try-calmer added the enhancement New feature or request label Sep 8, 2021
@why-not-try-calmer
Copy link
Collaborator Author

Requesting your opinion @KaratekHD :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant