Merge lp://staging/~sinzui/launchpad/poimport-typeerror into lp://staging/launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 16261
Proposed branch: lp://staging/~sinzui/launchpad/poimport-typeerror
Merge into: lp://staging/launchpad
Diff against target: 141 lines (+82/-22)
2 files modified
lib/lp/translations/model/pofile.py (+24/-21)
lib/lp/translations/tests/test_pofile.py (+58/-1)
To merge this branch: bzr merge lp://staging/~sinzui/launchpad/poimport-typeerror
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+133713@code.staging.launchpad.net

Commit message

Ensure the error data is sane enough to send an error email.

Description of the change

The TypeError happens when the script is trying to log an error. All the
data is sanitised except for the call to
potmsgset.getSequence(self.potemplate) to get the sequence. This can be
None if the TTI is incomplete. We just want to ensure the data is sane
enough so that we can log the error.

--------------------------------------------------------------------

RULES

    Pre-implementation: stevenk, jcsackett
    * Extract the logic that makes the error email to a separate testable
      method.
    * Add a test for the current behaviour.
    * Update the method to fallback to -1 if the sequence is None
    * Revise the loop to avoid needless string concatenation of errordetails

QA

    * Untestable, the case happens when the object is in an invalid state.

LINT

    lib/lp/translations/model/pofile.py
    lib/lp/translations/tests/test_pofile.py

LoC

    I have a credit of 4000 lines.

TEST

    ./bin/test -vvc -t prepare_pomessage lp.translations.tests.test_pofile

IMPLEMENTATION

Extracted the logic to a method and added a test. Added a rule to use -1
of the sequence is None.
    lib/lp/translations/model/pofile.py
    lib/lp/translations/tests/test_pofile.py

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Curtis,

Typo: enail

When there is no sequence will the users understand '-1'? Would another string like "Invalid sequence" be better?

Overall it looks good.

review: Approve (code)
Revision history for this message
Curtis Hovey (sinzui) wrote :

The emails are often parsed by scripts and we want to keep using the number. This is case though where there is nothing wrong with the sequence in the pofile...Lp is bad, not the pofile.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.