Page MenuHomeSoftware Heritage

Log error message when that occurs
ClosedPublic

Authored by ardumont on Nov 19 2020, 5:13 PM.

Details

Summary

Currently, we have nothing and it's not easy to determine the issue when
reading through logs. Logging such errors will help analyze when something is
wrong.

Closes T2626

Test Plan

tox

Diff Detail

Repository
rDDEP Push deposit
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D4536 (id=16071)

Rebasing onto 85416fb165...

Current branch diff-target is up to date.
Changes applied before test
commit 70a649f9d4fd7bc4d73b0f23859c40ed0d0cac03
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 19 17:13:38 2020 +0100

    Log error message when that occurs
    
    Currently, we have nothing and it's not easy to determine the issue when
    reading through logs. Logging such errors will help analyze when something is
    wrong.
    
    Closes T2626

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/397/ for more details.

Thanks!

Even though that's probably not a problem in that case, in general, please avoid f-strings or manual interpolation in logging statements, but rather use the built-in expansion which uses the extra arguments passed to logger.info. This avoids some (potentially costly) string interpolation when the logger is not enabled.

that, and I suggest a more compact formatting:

logger.info("%s %s -> %s('%s'):\n%s", request.method, request.path, exception.key, exception.summary, exception.verbose_description)

Even though that's probably not a problem in that case, in general, please avoid f-strings or manual interpolation in logging statements, but rather use the built-in expansion which uses the extra arguments passed to logger.info. This avoids some (potentially > costly) string interpolation when the logger is not enabled.

But... but why did i do that?!
Yes, i don't do that... usually...

Thanks for poking me.

Build is green

Patch application report for D4536 (id=16082)

Rebasing onto 85416fb165...

Current branch diff-target is up to date.
Changes applied before test
commit cb733dd49ab69f1e8ae32a4ed406d8a86284503f
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Nov 19 17:13:38 2020 +0100

    deposit.errors: Log error messages when that occurs
    
    Currently, we have nothing and it's not easy to determine the issue when
    reading through logs. Logging such errors will help analyze when something is
    wrong.
    
    Closes T2626

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/398/ for more details.

This revision is now accepted and ready to land.Nov 20 2020, 9:50 AM