fermi-ad / acsys-python

Python module to access the Fermilab Control System
MIT License
8 stars 4 forks source link

Allow hardcoded values to be overridden #43

Closed tiradani closed 2 years ago

tiradani commented 2 years ago

After reviewing the code surrounding the lifecycle of sockets, and looking at the issue discussing the idea of wrapping the sockets with SSL, I realized that there are parameters that are hard-coded, such as the proxy host name, the port, and some timeouts. If the socket is wrapped in SSL, at some point ciphers will need to be specified. Due to the changing landscape of ciphers and which ciphers are still considered safe, it may be advisable to create a configuration object for the DPM clients. The configuration object could have sensible defaults associated with it so that a config file is not required. The config file would override the defaults. This would allow for cipher changes to be specified without recompiling or repacking and redeploying code.

A topic for discussion: Whether or not any of the hard-coded values should appear anywhere in code, even as defaults. The clients would require a config file in that case, but that may not be a bad thing. However, the least disruptive path to using configurable values is to make them defaults and override the values in the config.

beauremus commented 2 years ago

Because this is a client library, we should support overriding configuration through our exposed interface object(s). This can be in addition to a config file, but this library is meant to be used programmatically and not always as a service. I think there are use cases for sane defaults, programmatic configuration, and static configuration in a file.

beauremus commented 2 years ago

I stumbled across this article which could be relevant. https://florian-dahlitz.de/articles/how-to-work-with-config-files-in-python

rneswold commented 2 years ago

I offer the following for discussion:

In my experiences with TOML, (in the FIRUS project), I set up sections in which one can be selected by a command line option. Something like this:

[default]
host = "acsys-proxy.fnal.gov"

[config.operational]
port = 6802

[config.test]
port = 6805

[config.experimental]
host = "virt01.fnal.gov"
port = 6802

In this example:

rneswold commented 2 years ago

If it helps to visualize, translating my previous post's example into JSON:

{
    "default": {
        "shot": "acsys-proxy.fnal.gov"
    },
    "config": {
        "operational": {
            "port": 6802
        },
        "test": {
            "port": 6805
        },
        "experimental": {
            "host": "virt01.fnal.gov",
            "port": 6802
        }
    }
}
tiradani commented 2 years ago

Based on the comments and discussion I will create a config class that is loaded when the library is loaded. The class will contain sensible defaults and expose them as properties. Config values that should be protected (as of this writing there are none known) will have the property decorator, but not the setter decorator.

Additionally, the config object will have the concept of environments baked in. An environment may have unique values for each of the properties in the config. Selecting the environment will automatically set the appropriate defaults for the config properties for that environment. Selecting environments multiple times will cause the last environment's values to be set.

The config object will have a dump method which will express the current values in the format specified, e.g., json, toml, etc.

Anything missing?

beauremus commented 2 years ago

Closed by #48