Use injector to initialize storage adaptor type #916
Conversation
| @@ -159,7 +160,13 @@ def get_namespace( | |||
| return CodebaseNamespace.from_db(db_namespace) | |||
|
|
|||
| @classmethod | |||
| def load_workspace(cls, namespace_id: int, skip_copy: bool = False): | |||
| @inject | |||
There was a problem hiding this comment.
Because we use injected we have to decorate the method with @inject OR @module.provider.
The difference is wether this method is a singleton that takes no required arguments, or a helper method that takes required arguments. Because this method is not a singleton, that is, it takes in arguments and runs differently based on the inputs, it uses inject.
|
|
||
| def __init__(self, repo_id: int, namespace_slug: str): | ||
| @inject | ||
| def __init__(self, repo_id: int, namespace_slug: str, app_config: AppConfig = injected): |
There was a problem hiding this comment.
Creating a StorageAdaptor now "gets" the AppConfig for free here.
| storage_type = os.getenv("CODEBASE_STORAGE_TYPE", "filesystem") | ||
|
|
||
| autofix_logger.debug(f"Using storage type: {storage_type}") | ||
| @module.provider |
There was a problem hiding this comment.
This is a module.provider because it is instantiated just once and then provided to other things as injected.
If the return value of a provider is going to be injected elsewhere, it is a @module.provider.
| ParsePath = Annotated[str, BeforeValidator(as_absolute_path)] | ||
|
|
||
|
|
||
| class AppConfig(BaseModel): |
There was a problem hiding this comment.
Helpful class that parses and types all of our os.environ inputs in one clean place. We can use pydantic features, including validators and post init, to catch configuration issues easily.
In the future, I intend to use this to check if your current .env file needs further values, making development more straight forward and automated.
| @@ -76,11 +77,23 @@ def from_annotation(cls, source: Any) -> "FactoryAnnotation": | |||
| not inner.is_collection | |||
| ), f"Cannot get_factory {source}: collections must be of concrete types, not other lists" | |||
| return dataclasses.replace(inner, is_collection=True) | |||
| elif annotation.origin is type: | |||
There was a problem hiding this comment.
This is support for factories that themselves return type[X].
|
|
||
|
|
||
| class TestStorageAdapter(unittest.TestCase): | ||
| def test_fs_workspace_clear(self): | ||
| os.environ["CODEBASE_STORAGE_TYPE"] = "filesystem" |
There was a problem hiding this comment.
Don't need to this kind of manipulation, see above how we set an override for stub_module that will be used by default in tests.
|
|
||
|
|
||
| @stub_module.provider | ||
| def provide_test_defaults() -> AppConfig: |
There was a problem hiding this comment.
We can create defaults for tests here, so that each test doesn't have to know how to create useful fake defaults.
AppConfigobject that can parse os.environ on app bootup, consolidating our various environment variable usages.get_storage_adaptor_classis a now amodule.providerthat receives theAppConfigobject viainjectedand selects its implementation based on that.I will be making more refactorings that integrate
AppConfigin other places to simplify our configuration and bootup sequence.