Differential D12

Set log level to 'error' for errors in the blocker proposal form
ClosedPublic
Actions

Authored by mkrizek on Feb 21 2014, 12:01 PM.

Details

Reviewers
tflink
Maniphest Tasks
T24: Insufficient logging for bug proposal form issues
Commits
rBLKR5009ea576e89: Set log level to 'info' for errors in the blocker proposal form
Summary

Fixes T24

Test Plan

Tested on my dev machine

Diff Detail

Branch
feature/T24-proposal-logging
Lint
No Linters Available
Unit
No Unit Test Coverage
tflink added a comment.Feb 24 2014, 1:04 AM

The default non-dev log level is info. I don't see a reason to raise these messages to ERROR, INFO should be enough to get the messages logged so that we can get a better idea of what's going wrong in the cases reported.

blockerbugs/controllers/main.py
416

I'm not sure this needs to be error, info should still show up in the logs. Changing the other catch-all form validation error to info should be sufficient

418

for the sake of being explicit, this is the catch-all logging message I was referring to. Changing this to info should be sufficient.

mkrizek added a comment.Feb 24 2014, 1:40 PM

I went ahead, made the change and pushed to develop - 49133a5abf4eca40169318d655c310f1ad9df71d

tflink accepted this revision.Feb 24 2014, 4:16 PM

The actual changes are good, hoping that this can be closed if it's marked as accepted

mkrizek closed this revision.Feb 24 2014, 4:27 PM

Revision Contents

Diff 27

View Options

blockerbugs/controllers/main.py

Show First 20 Lines • Show All 407 Lines • ▼ Show 20 Line(s)407 app.logger.info('%s proposing %i as %s for %s' % (
408 user, bugid, proposer.get_tracker_type(), selected_milestone.name)) 408​ user, bugid, proposer.get_tracker_type(), selected_milestone.name))
409 try: 409​ try:
410 proposer.propose_bugs(user, selected_milestone.name, bugform.justification.data) 410​ proposer.propose_bugs(user, selected_milestone.name, bugform.justification.data)
411 return render_template('thanks.html', bugid=bugid, 411​ return render_template('thanks.html', bugid=bugid,
412 isblocker=bugform.blocker.data, 412​ isblocker=bugform.blocker.data,
413 isfe=bugform.freeze_exception.data) 413​ isfe=bugform.freeze_exception.data)
414 except BugzillaError, e: 414​ except BugzillaError, e:
415 bugform.bugid.errors = [e.msg] 415​ bugform.bugid.errors = [e.msg]
416 app.logger.info('bug proposal form errors: %s' % str(bugform.errors)) 416​ app.logger.error('bug proposal form errors: %s' % e.msg)
tflinkUnsubmitted
Not Done

I'm not sure this needs to be error, info should still show up in the logs. Changing the other catch-all form validation error to info should be sufficient

417 else: 417​ else:
418 app.logger.debug('bug proposal form errors: %s' % str(bugform.errors)) 418​ app.logger.error('bug proposal form errors: %s' % str(bugform.errors))
tflinkUnsubmitted
Not Done

for the sake of being explicit, this is the catch-all logging message I was referring to. Changing this to info should be sufficient.

419 419​
420 # this assumes that the current milestone is the release against which the 420​ # this assumes that the current milestone is the release against which the
421 # blocker is being proposed. It isn't an ideal assumption but it should hold 421​ # blocker is being proposed. It isn't an ideal assumption but it should hold
422 # for most use cases. 422​ # for most use cases.
423 423​
424 current_release = get_current_milestone().release.number 424​ current_release = get_current_milestone().release.number
425 return render_template('propose_bug.html', title='Propose a Blocker or Freeze Exception', 425​ return render_template('propose_bug.html', title='Propose a Blocker or Freeze Exception',
426 bugform=bugform, release=current_release) 426​ bugform=bugform, release=current_release)
View Options

blockerbugs/templates/propose_bug.html

This file was changed only by adding or removing whitespace. Show File Contents