Owen-CH-Leung commented on PR #32781:
URL: https://github.com/apache/airflow/pull/32781#issuecomment-1647298444
> > @Adaverse Thanks for your feedback.
> > Yes I agree that we can re-use `import_string` and `configure_logging`
as suggested, but I'm a bit hesitant to modify the `configure_logging()` to
return the overriden `base_log_dir` since I saw it's being used in a few other
places also. (e.g. `airflow/www/app.py`, `airflow/settings.py` ,
`airflow/cli/commands/internal_api_command.py` and so on)
> > How about I just reuse these 2 functions and continue my override logic
here in `serve_logs.py` ? WDYT ?
>
> Sure looks good! Also it would be good to have these changes covered by
tests.
Sorry after inspecting the code a bit I'll have to remove the use of
`configure_logging`. The reason being this method will by default load the
`DEFAULT_LOGGING_CONFIG` defined in `airflow_local_settings.py` , and when it's
being called here, it will always override the setting at `base_log_dir` under
`logging` section. (And hence some of the tests are broken). I've fall back to
use `conf.get("logging", "logging_config_class")` but keep `import_string()`
for now.
Meanwhile, I'm adding some tests to cover the changes I made.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
