Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More convenient module configuration system #31

Open
cbgbt opened this issue Jul 4, 2014 · 9 comments
Open

More convenient module configuration system #31

cbgbt opened this issue Jul 4, 2014 · 9 comments

Comments

@cbgbt
Copy link
Collaborator

cbgbt commented Jul 4, 2014

A config system should exist such that modules which require configuration can automatically be passed a config object at init-time.

Perhaps if a module wishes to use this system, it could include a file in its root directory, sirc-config.default.[extension] If this file exists, the bot automatically places [extension-name].config.[extension] into the conf dir, this file is an exact copy of the default given by the module.

The module then must provide a configuration parsing mechanism. Perhaps

module.exports.parseConfig = function(configString, configFileName, callback)

If no such method is provided, but a default config is presented, the module manager simply tries to parse it as JSON. If it fails, it ignores errors and proceeds as normal (perhaps passing an empty config into the module init function)

@cbgbt cbgbt changed the title Easier configuration Easier module configuration system Jul 4, 2014
@cbgbt cbgbt changed the title Easier module configuration system More convenient module configuration system Jul 4, 2014
@euank
Copy link
Member

euank commented Jul 4, 2014

I like the parseConfig idea, but I would also like to support more format right off the bat.

Ideally, the following 4 formats would work with no parseConfig method provided:

  • config.json
  • config.js (basically a json file with weaker keys)
  • config.yml
  • config.ini (less important)

We can make this config format sorta module-manager specific. The bot should read the config file and provide it as a string to any module manager that wants it.

The module manager should then be able to add its own functionality on top of that (presumably almost all module managers will implement those formats excluding .js because that's a dumb format for non-node languages).

@cbgbt
Copy link
Collaborator Author

cbgbt commented Jul 4, 2014

Hmm. The way my implementation is right now, the module manager detects the existance of sirc-config.default.(extension) in the module folders. Are you saying that the core bot should be digging around in the module folder?

@euank
Copy link
Member

euank commented Jul 4, 2014

No, the core bot absolutely shouldn't and that's a good point.

However, the config folder, like the data folder, has to be centralized. For example, two modules in different languages each might want to login to twitter, and it would make sense for them to use the same config / account via saying "I want twitter.json".

The default config is a much more iffy thing and potentially each module manager could let modules bring their own, but that could lead to duplication. I'm not sure on that one, and I'll be okay with whatever design decision you want to go with I think.

@cbgbt
Copy link
Collaborator Author

cbgbt commented Jul 4, 2014

Maybe instead of dropping a magic file into a module and using an exported function, we could utilize whatever package system is implemented via #30

{
    ...
    "requires-config": [
        {
            "filename": "twitter.json",
            "parser": "module/twitter" // Load the config however module/twitter does it
        },
        {
            "filename": "my-config.ini", // The filename of the config dropped into the conf folder
            "default": "sirc-default.config.ini", // If no config exists, copy this one
            "parser": "default" // Default parser resolution. Try parseConfig, if none exists, fallback on attempting to detect extension and using the default parsers
        }
    ]
    ...
}

I guess my implementation currently might be a little too module manager-specific.

@euank
Copy link
Member

euank commented Jul 4, 2014

Looks good to me

@cbgbt cbgbt added duplicate and removed duplicate labels Jul 6, 2014
@cbgbt
Copy link
Collaborator Author

cbgbt commented Jul 6, 2014

I thought about this a little more.

Why not just export the needed config files?

module.exports.configsNeeded = [
    {
        "filename": "twitter.json",
        "parser": "default"
    }
]

@euank
Copy link
Member

euank commented Jul 6, 2014

Yeah, that's also a fine option. That was my original thought. Infer config name if it's not exported, use exported only if it is.

I don't have a strong preference regarding that vs shoving it in package.json.

@cbgbt
Copy link
Collaborator Author

cbgbt commented Jul 6, 2014

So look for (modulename).(predefined list of extensions) if there's no exported list, and do nothing if that file isn't there?

@euank
Copy link
Member

euank commented Jul 6, 2014

Yup!

And then set this.config if you find the modulename one, set this.config.[name] for each named config if they're named. Something like that.

Oh, and my preferred code off the top of my head for specifying:

module.exports.configFile = "x" // x.json, x.yml, who cares. Extension optional
module.exports.run = function() {
// this.config is set to x.json's parsed contents
}


// other module
module.exports.configFiles = ['x', 'y', 'z']

module.exports.run = function() {
// this.config.x, this.config.y, and this.config.z are set
}


// another module
module.exports.name = "sirc-example"

module.exports.run = function() {
//this.config is set to the contents of sirc-example.json if it exists
}

// another, custom parser

module.exports.configFiles = [
'x', // Default parser
{
    name: 'y.ini',
    parser: function(rawIn, parsedCallback) { /* todo */ }
}];

The intent here is to make handling single config files very clean because that is the general case, while allowing multiple as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants