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

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
101

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.

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.

This revision is now accepted and ready to land.Jan 9 2019, 2:24 PM
This revision was automatically updated to reflect the committed changes.