Skip to content

Avoid linting failures by unconditionally defining a logger property.#328

Closed
albu-diku wants to merge 1 commit intonextfrom
refactor/logger-Configuration-property
Closed

Avoid linting failures by unconditionally defining a logger property.#328
albu-diku wants to merge 1 commit intonextfrom
refactor/logger-Configuration-property

Conversation

@albu-diku
Copy link
Copy Markdown
Contributor

@albu-diku albu-diku commented Sep 6, 2025

Recent changes to the linting checks have resulted in any changes in and around configuration fail in CI with the complaint that logger is not defined. In trying to understand what was happening it was found that some amount of confusion was occurring having both logger and logger_obj properties. Attempt to fix this by:

  1. unconditionally defining both properties
  2. always setting both properties
  3. determining the type of logger being assigned and set the internal
    properties as appropriate

Expanding on the latter, loggers are almost always set as assignment to .logger, but this was not always being passed the same kind of object. At times this was a bare logger (i.e. info(), .debug() etc) but sometimes it was something with .reopen() which would then simply be thrown away and thus reload() would not actually work. Fix this by detecting a .reopen() method and correctly referencing such an object.

Recent changes to the linting checks have resulted in any changes in and
around configuration fail in CI with the complaint that logger is not
defined.

In trying to understand what was happening it was found that some amount
of confusion was occurring having both logger and logger_obj properties.
Attempt to fix this by:
1) unconditionally defining both properties
2) always setting both properties
3) determining the type of logger being assigned and set the internal
   properties as appropriate

Expanding on the latter, loggers are almost always set as assignment to
.logger, but this was not always being passed the same kind of object. At
times this was a bare logger (i.e. info(), .debug() etc) but sometimes it
was something with .reopen() which would then simply be thrown away and
thus reload() would not actually work. Fix this by detecting a .reopen()
method and correctly referencing such an object.
@albu-diku
Copy link
Copy Markdown
Contributor Author

Attempted fix to #328.

@albu-diku
Copy link
Copy Markdown
Contributor Author

Note: I think this cleanup remains worthwhile, but it might be much better done as follow on work to the runtime and static config split (#467).

Specifically, the logger is a runtime property and thus needs to be moved up a level. The initial split does not do this, but it will need to happen, and at that point we can use the ideas here to ensure that property is always defined and its lifecycle managed correctly.

@albu-diku albu-diku closed this Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant