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

Allow configuration of piwik plugin using a configuration file #182

Open
arturo-seijas opened this issue Jul 5, 2022 · 9 comments
Open

Comments

@arturo-seijas
Copy link

The default configuration options for piwik can not be overwritten by adding an entry in the configuration file indico.conf unlike the customizable settings for Indico.

I think it would be a nice to have, since it will allow having this configuration defined as code . The only working approach at this time is to set them manually through the UI.

@ThiefMaster
Copy link
Member

This doesn't really fit well together in the schema of settings that are managed via the UI: They are all stored in the database and have UI where the user can change them, so reading them from a config file (or env vars) would be a bit strange - they'd either need to always override the DB settings or somehow be imported into the DB (and updated if they change at a later time).

Not sure that's a good solution for this tbh...

@arturo-seijas
Copy link
Author

How about the timezone, for instance. That's the approach I had in mind when I opened the ticket. Right now it's an UI setting but can also be defined with the DEFAULT_TIMEZONE environmental variable and hence, on a configuration file. Another example would be the S3 plugin, that can also be configured using this approach.

With this change we could leverage the configuration as code advantages also for this plugin.

@ThiefMaster
Copy link
Member

DEFAULT_TIMEZONE cannot be fully overridden in the UI; you can only set a different time zone for specific categories or events..

@arturo-seijas
Copy link
Author

How about disabling UI edition if the config option is specified using an environmental variable? I've seen this approach around and that would clearly establish priorities.

@ThiefMaster
Copy link
Member

ThiefMaster commented Aug 23, 2022

So I was thinking about this, and introducing individual env vars seems like a bit of a mess (especially considering data types - settings are stored as JSON after all!).

So my suggestion (ideally for you to contribute) would be to add an indico.conf setting SETTINGS_OVERRIDE that can be set to a dict like this:

SETTINGS_OVERRIDE = {
    'plugin_piwik': {
        'server_api_url': 'https://example.com'
    }
}

In the get_all_settings and get_setting util you then add code that takes this config setting into account and instead of getting the value from the DB, the one from the override will be used.

The documentation for this setting would clearly mention that it's an advanced feature and module/setting names need to be looked up in the code if someone wants to use it.

As a bonus, the plugin settings form rendered from RHPluginDetails could disable the form fields for any setting locked like this so an admin doesn't try to change it via the web UI.

@arturo-seijas
Copy link
Author

That's sounds like a nice solution for what we're looking for :)

@ThiefMaster
Copy link
Member

Just to avoid any misunderstanding and both sides waiting for the other to add it: Are you willing to contribute this?
It should be pretty straightforward and while the two utils I mentioned in my last comment don't have their own unit tests, the SettingsProxy-related tests should probably cover it well enough.

@arturo-seijas
Copy link
Author

We should be able to work on this at some point, probably in a couple of weeks. Does that sound good to you?

@ThiefMaster
Copy link
Member

Sure! Sounds like something for 3.2.1 or 3.2.2 then ;)

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

No branches or pull requests

2 participants