Manager class for applications using Task API #4657
Conversation
Stores application definitions and state (enabled/disabled). Uses EnvironmentManager to enforce that the appropriate environment is available and enabled before enabling an application. Definitions can be loaded from JSON files. Signed-off-by: Adam Wierzbicki <awierzbicki@golem.network>
Codecov Report
@@ Coverage Diff @@
## develop #4657 +/- ##
===========================================
+ Coverage 89.15% 90.29% +1.14%
===========================================
Files 224 225 +1
Lines 20208 20264 +56
===========================================
+ Hits 18016 18298 +282
+ Misses 2192 1966 -226 |
| if app_name not in self._apps: | ||
| raise ValueError(f"Application '{app_name}' not registered.") | ||
| env_id = self._apps[app_name].requestor_env | ||
| if not self._env_manager.enabled(env_id): |
There was a problem hiding this comment.
EnvManager is only required for this single sanity check, I wonder if it's justified to make it a dependency. Especially that you can disable the environment just after enabling the app, thus introducing "invalid" state anyway. Maybe just performing these checks in the RequestedTaskManager when necessary is better.
There was a problem hiding this comment.
My reasoning is basically that if you don't allow to enable an app when the env is disabled you should disable all the apps when disabling the env as well - which we're not doing. It'd be circular dependency meaning that these classes should be merged, which doesn't make sense. Thus removing this check makes sense to me.
Stores application definitions and state (enabled/disabled). Uses EnvironmentManager to enforce that the appropriate environment is available and enabled before enabling an application. Definitions can be loaded from JSON files.