Allowing log_config to accept ConfigParser instance or a file object

[antonymayi]

The existing logic for the log_config option accepts:

• dict or str representing a JSON or YAML file name to be parsed using logging.config.dictConfig
• str representing an INI file name to be parsed using logging.config.fileConfig

The logging.config.fileConfig, however, accepts not only the file name, but possibly also an instance of configparser.RawConfigParser and even an open file object. Passing the instance of configparser.RawConfigParser (or its subclass) would be especially useful as this can accumulate configs from multiple files (cp.read() can be called repeatedly on different files updating its config values) instead of just a single one.

I am proposing this trivial change to allow the log_config option to also accept the *ConfigParser instance or a general file object.

[Kludex]

I didn't even know about ConfigParser 🤣

Anyway... Can you show me an example on how you use this? A self-contained repository that uses this branch is enough.

Also, we need to add a test for it. 👀

[antonymayi]

re the usage example:

I use uvicorn as a specific plugin within the ForML framework which itself is using its own logging config potentially combining multiple user-provided INI files loaded (via the LOGGING.read()) into a single ConfigParser instance.

It would be convenient to be able to pass that instance to the uvicorn setup as shown here (working example based on my PR).

I've updated the PR with the test_log_config_file now covering not just a string file name but also an instance of a ConfigParser and a file-like object.

[antonymayi]

Hi @Kludex, happy to update the PR based on your further guidelines in case you have any?

[iudeen]

I like this! 👍

[Kludex]

Is there any big project that uses ConfigParser?

[antonymayi]

Is there any big project that uses ConfigParser?

Depends on the definition of a big project but I believe Apache Airflow for instance should be a fit beyond any doubt.

[Kludex]

What is the amount of work needed for you if this PR is not accepted?

[antonymayi]

What is the amount of work needed for you if this PR is not accepted?

Making my life better or worse shouldn't be the concern here (even though I appreciate the consideration).

Can you please briefly clarify your reservations about this feature? Do you see any issues or do you just think it is a marginal use case and hence not worth it?

The way I see it, this really makes it way easier to integrate uvicorn logging within any embedding project. Without it, uvicorn is forcing the embedding application to use a single physical file (why single? why physical?) or essentially a hand-written (since there is no standard API to produce it programmatically) dictionary. This seems unnecessarily too demanding from a component within an embedding project (especially if it is not even a central component like in my case).

Had uvicorn supported just ConfigParser and not any of the other current options (files/dict), it would still have supported all these usecases (it is possible to create ConfigParser instance from files/dict) but not the other way round. So ConfigParser should have imho always been the primary option and only then considering the other ways (explicit files/dict) as convenient cosmetical shortcuts.

[antonymayi]

I accidentally deleted the forked repository which automatically closed this PR. I am opening a new one as #1976, apologies for the inconvenience.