Log error message when that occurs
ClosedPublic
Actions

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

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Maniphest Tasks

T2626: deposit: Improve log rendering to at least log the reason behind error codes

Commits

rDDEPcb733dd49ab6: deposit.errors: Log error messages when that occurs

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

ardumont created this revision.Nov 19 2020, 5:13 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptNov 19 2020, 5:13 PM

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.

Harbormaster completed remote builds in B17273: Diff 16071.Nov 19 2020, 5:16 PM

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.

Thanks dudes.

Adapt according to review

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.

Harbormaster completed remote builds in B17282: Diff 16082.Nov 20 2020, 9:49 AM

vlorentz accepted this revision.Nov 20 2020, 9:50 AM

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

Closed by commit rDDEPcb733dd49ab6: deposit.errors: Log error messages when that occurs (authored by ardumont). · Explain WhyNov 20 2020, 10:04 AM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDDEPcb733dd49ab6: deposit.errors: Log error messages when that occurs.