Fix stack overflow triggered by casefolding script

[Azrenbeth]

Fix stack overflow triggered by casefolding script (added in commit 5f90f62)

Logging was setup too many times. This adds option to ignore logging config options when parsing config files (so you can do that seperately)

[richvdh]

I've said this elsewhere, but I'm super dubious about having parse_config_file set up logging anyway. Can we just leave logging unconfigured (or better, configured to a hardcoded default which just writes the logs to stderr) until parse_config_file completes, and then call setup_logging from sydent.py ?

[Azrenbeth]

I've said this elsewhere, but I'm super dubious about having parse_config_file set up logging anyway. Can we just leave logging unconfigured (or better, configured to a hardcoded default which just writes the logs to stderr) until parse_config_file completes, and then call setup_logging from sydent.py ?

I agree that this wasn't a great setup.. I've now added a setup_logging_from_file function, that you can call before parse_config_file (so that you can use logging while parseing the config file)

[richvdh]

Parsing the config file twice seems a bit grim. I'm not really following why it's so important to fully configure the logging before we parse the config file for real?

[Azrenbeth]

Parsing the config file twice seems a bit grim. I'm not really following why it's so important to fully configure the logging before we parse the config file for real?

My thought was: it's a bad thing for output to not all go to the same place.

Say someone had configured Sydent (running in a docker container for example) and had setup something to follow the logs, wouldn't it be confusing if some output was missing? E.g. the warning telling the user to setup their server name to something more sensible.

A better way to do what I'm trying to do might be to have logging configured in a seperate file.

[richvdh]

Would it not be a bad thing for not-all output to go to the same place? So say you'd configured Sydent to run (e.g. in a docker container) and had setup something to follow the logs, wouldn't it be confusing if some output went somewhere else?

A docker container is probably a bad example: in that case, it's normal to have your application send everything to stdout/stderr, and let the docker runtime deal with it.

If you're not running in a docker container, then I think it's reasonable for errors that happen during startup (and thus prevent proper startup) to go to the console, while later errors go to a logfile. That's what most server processes do. Indeed, if errors during startup only go to a logfile, then it's pretty confusing when you start the process, since you can't tell if it started successfully or not.

[Azrenbeth]

If you're not running in a docker container, then I think it's reasonable for errors that happen during startup (and thus prevent proper startup) to go to the console, while later errors go to a logfile. That's what most server processes do. Indeed, if errors during startup only go to a logfile, then it's pretty confusing when you start the process, since you can't tell if it started successfully or not.

Thank you for explaining! That makes a lot of sense. I've moved all the logging output from the config parsing to using print() and moved the setup_logging function into sydent.py

[richvdh]

lgtm!

[richvdh]

this isn't your fault, but this doesn't seem to be used anywhere 🤦

[Azrenbeth]

Oh I removed that at some point in a seperate PR but might as well do it here instead!

[richvdh]

it might have been better to leave it, and wire it up correctly, but never mind :)