Draft: Allow reading from environment variables for options

[poperigby]

I need to test this but it should work.

[pbiering]

please provide update for DOCUMENTATION.md and also if possible create some test cases

[poperigby]

@pbiering Will do. Where should I put the test, and also, would you like me to use defaultdict(dict) for arguments_config as well? Seems to be more elegant.

[pbiering]

ok

[pbiering]

ok

[pbiering]

@pbiering Will do. Where should I put the test

There is a test subdirectory, check whether you can copy+adjust one or more

[poperigby]

Not quite sure how to do the test. I need to set an environment variable before the test server starts.

[pbiering]

Not quite sure how to do the test. I need to set an environment variable before the test server starts.

potentially there is a way to set environment variable in Python before calling the function.

[poperigby]

The problem is that the setup function is called before any test is run I think. I was thinking of putting it in test_server.py.

[pbiering]

The problem is that the setup function is called before any test is run I think. I was thinking of putting it in test_server.py.

would it not possible to copy and adjust test_command_line_interface and instead of defining command line options, set environment by e.g. os.environ[option] = value before reaching the subprocess.Popen?

[poperigby]

@pbiering I think it should be good now if you wanna take a look.

[pbiering]

Generally it looks good but your test case leads me to that it would be not safe for options which have _ inside.

My original proposal was like RADICALE_OPTION_<section>__<key>="<value>"

And looks like I had forgotten to mention the reason behind the __ (2x _) separator was that <section> + <key> must be able to support single _. Dash - cannot be used because not supported e.g. in bash.

Please check and adjust/extend code and also test cases like

# option without separator
RADICALE_OPTION_server__hosts="<value>"

# option with separator
RADICALE_OPTION_auth__htpasswd_filename="<value>"

[poperigby]

Made a PR

[pbiering]

ok

[pbiering]

@poperigby : code approved but tests partially failed, please investigate, thank you!

[poperigby]

@pbiering That's bizarre I don't remember making the comment about making a PR. Must have been a mistake. As for using a double underscore, yeah I'll fix that. The test seems to be failing because the test environment is using an older version of Python that doesn't have removeprefix()?

[pbiering]

@poperigby : yes, removeprefix() was introduced with Python 3.9.0, but it's currently not planned to drop support for 3.8.0 already with Radicale 3.3.0 - can you try to circumvent this by catching the used Python version and create for < 3.9 a workaround code sniplet?

if sys.version_info >= (3, 9):
    ...current code...
else:
    ...workaround code...

[pbiering]

@poperigby while working on another issue it came into my mind why not using the command line options which fully support all all options of the config file and have precedence.

There are two ways support, example:

## simple variable
RADICALE_OPTIONS="--logging-level=debug --config=/etc/radicale/config --logging-request-header-on-debug --logging-rights-rule-doesnt-match-on-debug"; /usr/bin/radicale $RADICALE_OPTIONS

## as array
RADICALE_OPTIONS=("--logging-level=debug" "--config=/etc/radicale/config" "--logging-request-header-on-debug" "--logging-rights-rule-doesnt-match-on-debug"); /usr/bin/radicale ${RADICALE_OPTIONS[@]}

Can this also help in your case, because then no extension would be required at all.

[poperigby]

Yeah that's definitely an option, although it's certainly not as nice looking or easy to write as environment variable versions of options.

[pbiering]

Yeah that's definitely an option, although it's certainly not as nice looking or easy to write as environment variable versions of options.

there is also a 3rd method

RADICALE_OPTIONS=()
RADICALE_OPTIONS+=("--logging-level=debug")
RADICALE_OPTIONS+=("--config=/etc/radicale/config")
/usr/bin/radicale ${RADICALE_OPTIONS[@]}

requires no additional support and one can take the options directly from the online help and does not need to replace - by _

[pbiering]

@poperigby are you satisfied already with the existing capabilities as described? Then your PR would not be needed.

[poperigby]

Yes I suppose that would work.