proposal: refactor config to allow nested items

[alexandrevicenzi]

Hi,

The current configuration mechanism does not allow many useful features such as:

• Slice types
• Map types
• Nested objects
• Custom validation
• and more

I'm trying to create a new Processor, but the configuration is extremely limited, it does not parse JSON that has types that are not hardcoded in the reflection used in ExtractConfig. The current config is a bit messy and it's hard to indetify what config belongs to which processor.

I have a proposal, refactor the config area to parse a valid JSON and allow more robust configurations. My idea of configuration would look like:

{
    "log_file" : "stderr",
    "log_level" : "info",
    "backend_config": {
        "save_process" : "HeadersParser|Header|Debugger",
        "gw_save_timeout" : "30s",
        "gw_val_rcpt_timeout" : "3s",
        "process_config": {
            // processor name as key for processor config as they shoud be unique
            "debug": {
                "log_received_mails": true
            },
            "guerrillaredisdb": {
                "sql_driver": "",
                "sql_dsn": "",
                "redis_expire_seconds": "",
                "redis_interface": ""
            },
            "mycustomprocessor": {
                "key": {
                    "subkey": {
                        "anotherkey": true
                    }
                }
            }
        }
    },
    "servers" : []
}

To acomplish this a few things must happen:

• Use encode/json to deal with decoding, no hacks, no custom parser, no anything
• All configs must implement an interface, that has a method IsValid, and this method validates the config for its processor
• Backend calls IsValid for each processor that is going to be created
• Backend only initialize the processor if IsValid succeds, if not, fail server start

This proposal has one issue:

• It breaks compatility, v2 version would be required

I'm willing to:

• Send the PR with all changes in this repo
• Send a PR to processors under flashmob to upgrade to v2

Let me know what you think about, it's an interesting change I would say, and it requires code changes in many places.

[flashmob]

Hello,

Thanks, that's a great proposal!

Something like this has already been done in the "stream", https://github.com/flashmob/go-guerrilla/pull/135

The parsing hack was done only because old versions of Go did not have a helpful json error message. Newer versions of Go improved this so it was also marked for removal in this branch.

However, the biggest feature is that config has more improved structure, allowing for each server to have their own backend config,

Here's what was noted down:

1. named backend configurations (not just one)
2. Each named backend configurations can also defines its own named processors

3. Processors define their configuration options as fields of the named processor It would be useful for:

   • each server to have a specific "named" backend configuration
   • ability to have a specific instance of a processor for a specific backend config, eg. servers can use different databases
   • do not need to prefix the config options with the name of the processor anymore

Also, your suggestion of using a Validator interface could be good.

Work on this branch has paused, but will restart next week (lots of testing & debugging to do). It will eventually become "V2"

On Fri, 16 Oct 2020 at 04:48, Alexandre Vicenzi notifications@github.com wrote:

Hi,

The current configuration mechanism does not allow many useful features such as:

• Slice types
• Map types
• Nested objects
• Custom validation
• and more

I'm trying to create a new Processor, but the configuration is extremely limited, it does not parse JSON that has types that are not hardcoded in the reflection used in ExtractConfig. The current config is a bit messy and it's hard to indetify what config belongs to which processor.

I have a proposal, refactor the config area to parse a valid JSON and allow more robust configurations. My idea of configuration would look like:

{ "log_file" : "stderr", "log_level" : "info", "backend_config": { "save_process" : "HeadersParser|Header|Debugger", "gw_save_timeout" : "30s", "gw_val_rcpt_timeout" : "3s", "process_config": { // processor name as key for processor config as they shoud be unique "debug": { "log_received_mails": true }, "guerrillaredisdb": { "sql_driver": "", "sql_dsn": "", "redis_expire_seconds": "", "redis_interface": "" }, "mycustomprocessor": { "key": { "subkey": { "anotherkey": true } } } } }, "servers" : []}

To acomplish this a few things must happen:

• Use encode/json to deal with decoding, no hacks, no custom parser, no anything
• All configs must implement an interface, that has a method IsValid, and this method validates the config for its processor
• Backend calls IsValid for each processor that is going to be created
• Backend only initialize the processor if IsValid succeds, if not, fail server start

This proposal has one issue:

• It breaks compatility, v2 version would be required

I'm willing to:

• Send the PR with all changes in this repo
• Send a PR to processors under flashmob to upgrade to v2

Let me know what you think about, it's an interesting change I would say, and it requires code changes in many places.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/flashmob/go-guerrilla/issues/217, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE6MP33XF2PL7EBCGYOOQDSK5GYTANCNFSM4SSNTVUQ .