Circular dependency vs. a single root config.lua file

CSDUMMI commented

2023-02-20 14:34:48 +00:00

(Migrated from gitlab.com)

I'm facing a problem, that appears to have no nice solution.

We want to have a single config file that imports other configuration files (pipeline and actions) and combines them into a single table, which is then merged with the configuration table provided by the user.

The reason we haven't done this before is because it leads to a circular dependency when defining the pipeline using checks and importing the configuration table from checks.

flowchart TD

    config --> pipeline

    pipeline --> checks

    checks --> WholeWordCheck

    WholeWordCheck --> config

(The same circularity applies to all checks using global configuration variables)

We have four options to resolve this circular dependency problem:

Remove pipeline from the config table and have two independent configuration files
Remove checks from pipeline and use dependency injection
Remove config from WholeWordCheck and use dependency injection on new.
Remove global configuration from all checks.

The first option is undesirable, because it makes the configuration of instances more complicated (a far more common activity than implementing a new action/check).

The second option, too, makes modifying the default pipeline more complicated, because the pipeline would now have to return a function (taking checks as an argument) instead of simply returning a table. This is still a more common activity than implementing a new check or action.

The third option is more involved, because it means that a checks run function would have to accept the global configuration (we cannot use the new function because this is called when creating the pipeline and before a configuration table is available).

The fourth option has the big disadvantage that checks are limited to only using configuration provided to it directly and cannot access global config, which would mean that both the WholeWordCheck and DomainCheck could no longer be maintained, because they use the global additional_config_path variable. I'd have to remove them. Checks relying on web sources (SubscriptionsDomainCheck) will not be impacted by this change.

Whatever option we choose, I'd implement it both for actions and checks.

I'm facing a problem, that appears to have no nice solution. We want to have a single `config` file that imports other configuration files (`pipeline` and `actions`) and combines them into a single table, which is then merged with the configuration table provided by the user. The reason we haven't done this before is because it leads to a circular dependency when defining the pipeline using checks and importing the configuration table from checks. ```mermaid flowchart TD config --> pipeline pipeline --> checks checks --> WholeWordCheck WholeWordCheck --> config ``` (The same circularity applies to all checks using global configuration variables) We have four options to resolve this circular dependency problem: 1. Remove pipeline from the config table and have two independent configuration files 2. Remove checks from pipeline and use dependency injection 3. Remove config from WholeWordCheck and use dependency injection on `new`. 4. Remove global configuration from all checks. The first option is undesirable, because it makes the configuration of instances more complicated (a far more common activity than implementing a new action/check). The second option, too, makes modifying the default pipeline more complicated, because the pipeline would now have to return a function (taking `checks` as an argument) instead of simply returning a table. This is still a more common activity than implementing a new check or action. The third option is more involved, because it means that a checks `run` function would have to accept the global configuration (we cannot use the `new` function because this is called when creating the `pipeline` and before a configuration table is available). The fourth option has the big disadvantage that checks are limited to only using configuration provided to it directly and cannot access global config, which would mean that both the `WholeWordCheck` and `DomainCheck` could no longer be maintained, because they use the global `additional_config_path` variable. I'd have to remove them. Checks relying on web sources (SubscriptionsDomainCheck) will not be impacted by this change. Whatever option we choose, I'd implement it both for `actions` and `checks`.

CSDUMMI commented

2023-02-20 14:34:48 +00:00

(Migrated from gitlab.com)

assigned to @CSDUMMI

CSDUMMI commented

2023-02-20 14:34:54 +00:00

(Migrated from gitlab.com)

created branch 29-circular-dependency-vs-a-single-root-config-lua-file to address this issue

created branch [`29-circular-dependency-vs-a-single-root-config-lua-file`](/babka_net/activitycolander/-/compare/main...29-circular-dependency-vs-a-single-root-config-lua-file) to address this issue

CSDUMMI commented

2023-02-20 14:37:04 +00:00

(Migrated from gitlab.com)

changed the description

CSDUMMI commented

2023-02-20 14:43:50 +00:00

(Migrated from gitlab.com)

changed the description

CSDUMMI commented

2023-02-20 14:45:25 +00:00

(Migrated from gitlab.com)

changed the description