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

Detangle Config dependencies #659

Open
olevski opened this issue Feb 19, 2025 · 1 comment
Open

Detangle Config dependencies #659

olevski opened this issue Feb 19, 2025 · 1 comment
Labels
cooldown Issues to be tackled during cooldown

Comments

@olevski
Copy link
Member

olevski commented Feb 19, 2025

Curently the app_config module depends on essentially all other modules because it imports the types and classes for everything and combines everything into one place. Then the single huge Config class is pulled into the server and started. This is nice but it means that any module that imports from the app_config module depends on all other modules. Which is not nice and is putting us on the road to having circular dependencies.

We can improve this by:

  • having each component own its own config
  • move the large Config class to bases.data_api or just fully remove it and make each server compose its own large config object at startup
  • with the above two changes the app_config module is essentially non-existent or very little is left in there

With this we do not have the case where if you import anything from the app_config module then you have to also import a bunch of fully unrelated stuff due to the fact that app config depends on essentially ALL other modules.

@olevski olevski added the cooldown Issues to be tackled during cooldown label Feb 19, 2025
@leafty
Copy link
Member

leafty commented Feb 19, 2025

move the large Config class to bases.data_api or just fully remove it and make each server compose its own large config object at startup

If you do this, then repository classes will be instantiated multiple times, one for each blueprint which requires the class.

The app_config module depends on all other modules because it does dependency management. If you import any class from it, it should be a type-only import to not create any cycle.

There is the possibility of having each module own its config, and then to handle dependencies from other modules, a type-only import must be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cooldown Issues to be tackled during cooldown
Projects
None yet
Development

No branches or pull requests

2 participants