Draft: Refactor top level Python without strict dependency injection
What does this MR do?
This MR is a version of !248 (closed) with some of the dependency injection removed.
I'm not intending to merge this MR. I want to use it as a demonstration to help us decide whether or not to strictly use dependency injection.
Technical details
- Stop injecting Python modules
logging
anduuid
- Stop injecting
System
- In 3014c23e, provide an example of not injecting our own classes, and of only providing low level classes with the configuration options they need
What I learned
- It feels wrong to me to inject Python's own modules, for the following reasons:
a. We're already only injecting them sometimes
b. They are used at every level of the application, so injecting them from the top level (like we were withlogging
) adds a lot of boilerplate
c. We can trust that they will not be modified while the application is running
d. Even typing them is weird. For example,logging
is of typeModule
. It feels odd (unnecessarily meta) to be passing a builtin Python module through our classes
e. It could be argued that some, likelogging
, might be replaced at some point by a custom or third party library. Even if that is true, injecting it feels like pre-optimization. Moreover, we might want to use a different method for implementing a custom logger (like using a singleton). - It feels wrong to me to inject custom classes that we use at every level of the application and that we can trust not to be modified while the application is running, like
System
, for the same reasons I've written out for Python's own modules (except the typing one) - It feels nice to inject dependencies that we want to restrict to the top level, like
Configuration
andZAProxy
. For example, I'd rather pass only the specific configuration options needed to lower level classes to prevent them from having knowledge of unnecessary options. Having to inject the configuration explicitly serves as a reminder to me to instead inject only the specific options that are needed - I generally like not injecting our own classes without an explicit reason to do so. It makes the class hierarchy clearer to me and the code looks cleaner too
a. 3014c23e illustrates what the code would look like in a case where we don't inject the dependencies
What are the relevant issue numbers?
gitlab-org/gitlab#227749 (closed)
Does this MR meet the acceptance criteria?
-
Changelog entry added -
Documentation created/updated for GitLab EE, if necessary -
Documentation created/updated for this project, if necessary -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Job definition updated, if necessary -
Job definition example -
Vendored CI Templates (also in CE)
-
-
Conforms to the code review guidelines -
Conforms to the Go guidelines -
Security reports checked/validated by reviewer
Edited by Avielle Wolfe