Page MenuHomeSoftware Heritage

Log used config files.
ClosedPublic

Authored by vlorentz on Dec 21 2018, 1:43 PM.

Details

Diff Detail

Repository
rDCORE Foundations and core functionalities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Dec 21 2018, 1:43 PM
olasd requested changes to this revision.Dec 21 2018, 3:17 PM
olasd added a subscriber: olasd.
olasd added inline comments.
swh/core/config.py
102

That's an info at best, as it'd log an error message if a config file voluntarily doesn't exist (e.g. during tests).

exists_accessible already raises relevant errors when a file is present but inaccessible.

This revision now requires changes to proceed.Dec 21 2018, 3:17 PM

That's an info at best, as it'd log an error message if a config file voluntarily doesn't exist (e.g. during tests).

exists_accessible already raises relevant errors when a file is present but inaccessible.

We agreed that tests should not try to load config files. And allowing config files to be missing without reporting errors is asking for trouble.

olasd added a comment.Jan 9 2019, 2:13 PM

That's an info at best, as it'd log an error message if a config file voluntarily doesn't exist (e.g. during tests).

exists_accessible already raises relevant errors when a file is present but inaccessible.

We agreed that tests should not try to load config files.

Okay?

And allowing config files to be missing without reporting errors is asking for trouble.

All the systems currently in production depend on this behavior: they try to load local configuration (in ~/.config), which fails (expectedly!), then they load the configuration in /etc.

All in all I don't think all workers should immediately start spewing a handful of error messages at each run for something that is _expected_.

I'd even go as far as putting those messages at DEBUG level.

In D878#19188, @olasd wrote:

All the systems currently in production depend on this behavior: they try to load local configuration (in ~/.config), which fails (expectedly!), then they load the configuration in /etc.

Indeed, I forgot about that.

vlorentz updated this revision to Diff 2838.Jan 9 2019, 2:23 PM
  • Lower log levels.
olasd accepted this revision.Jan 9 2019, 2:24 PM
This revision is now accepted and ready to land.Jan 9 2019, 2:24 PM
vlorentz updated this revision to Diff 2839.Jan 9 2019, 2:26 PM
  • rebase / squash
This revision was automatically updated to reflect the committed changes.