Merge lp:~danilo/launchpad/bug-410579 into lp:launchpad
- bug-410579
- Merge into devel
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Barry Warsaw | ||||||||
Approved revision: | no longer in the source branch. | ||||||||
Merged at revision: | not available | ||||||||
Proposed branch: | lp:~danilo/launchpad/bug-410579 | ||||||||
Merge into: | lp:launchpad | ||||||||
Diff against target: | None lines | ||||||||
To merge this branch: | bzr merge lp:~danilo/launchpad/bug-410579 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Barry Warsaw (community) | Approve | ||
Review via email: mp+10179@code.launchpad.net |
Commit message
Description of the change
Данило Шеган (danilo) wrote : | # |
Barry Warsaw (barry) wrote : | # |
On Aug 14, 2009, at 06:35 PM, Данило Шеган wrote:
> Данило Шеган has proposed merging lp:~danilo/launchpad/
> bug-410579 into lp:launchpad/devel.
>
> We should include more information, like what the request is for, and
> how download link will expire in 1 week. We should also include
> information on how you can re-request the download if you need to.
Thanks for working on this Danilo. It looks like a very useful change
that
will help our users!
> Subject should not mention your username, but instead a short
> description of what the download is about.
>
> == Implementation details ==
>
> The branch has quickly gotten very big (~900 lines), but it's mostly
> because of textual changes to email templates. I hope it's not a
> problem.
>
> * scripts/
> This is where the core change is: everything related to emailing is
> moved into
> ExportResult implementation, which generates appropriate emails.
>
> Two methods are of notice:
>
> * _getExportReque
> most
> likely made;
> TranslationImpo
> POFile objects
> and depending this method figures out the place where the
> request was
> "most likely" made (i.e. context where you could have requested
> these
> files, but then trying for the most relevant context as well).
>
> For instance, if we've got two pofiles on a single template,
> request was
> likely made on a potemplate object, so we return that (for
> nicer URLs
> and emails subjects though, if there's only one template in
> a productseries/
>
> * _getShortReques
> previous method
> and returns a nice short description in the form of 'Evolution
> trunk' or
> 'Ubuntu Karmic gimp-2.0 - Spanish translation' suitable for use
> as email
> subjects
>
> These are not unit tested, though they could be, simply because
> they are
> sufficiently well exercised by existing doctests, and because if
> these start
> failing, it'll be easy to notice (so, I'd rather not increase CPU
> cycles
> we spend on tests). They are mostly for 'nicer UI output', so
> their correctness
> is not critical to the operation.
Do you really think it would take up that much more cycles to unit
test these
two methods? I'm generally not in favor of these kinds of indirect
test when
they can be reasonably avoided because that makes it much more
difficult to
change the methods later. You never really quite know what the effect
will
be, so you end up having to run the full test suite "just in case".
With a
unit test, you at least know you need to run that test if you change the
method, and can presume that if those unit tests pass, everything else
implicitly testing them will too.
>
> Other changes in this file are mostly for using emailtemplates
> instead of hardcoding text.
>
> * emailtemplates/
> All the email templates have been moved out of the code and into
> separate
> template files. Some 75 lines of diff are basically
>
> * doc/poexpor...
Данило Шеган (danilo) wrote : | # |
Barry, thanks for the review.
I've applied most of your recommendations. The ones I didn't were about
'positive tests' (i.e. is 'is not None' really a negative test?) and I
haven't provided unit tests. Still, I've modified doctests to use
mail_helpers, so that resulted in a nice big incremental diff.
У пет, 14. 08 2009. у 16:05 -0400, Barry Warsaw пише:
> > * doc/poexport-
> > doc/poexport-
> > doc/poexport-
> > There are a lot of changes to tests that test how emails are generated:
> > basically, half of the branch is there (440 lines). The changes there
> > include introducing a shared helper print_mail_
> > print subject along with body of the email, and changes to test new
> > email contents and subjects. Some minor changes to how ExportResult is
> > constructed were necessary as well to conform with the new API (i.e. pass
> > person, requested exports and logger).
>
> Do you know about lp.testing.
> that plus ellipses to cut down on the output. Is there a reason why you'd
> want print_mail_
Yes, I've looked through existing helpers. I've already struggled to
keep the diff size reasonable, and this would have made it only worse.
Though, I am happy to migrate existing tests to the use them, and I did
so. Note that only this change means 640 lines of diff in the
incremental diff ;)
> > To not increase size of the diff, some bits of the test are left
> > indented with 2 spaces. I'd be happy to fix before submitting.
>
> Sure, thanks! rs=me on those additional cleanups.
Thanks.
> For the most part, the branch looks fine. I do have a few questions and
> suggestions, but I'll mark this as merge-conditional on the hope that those
> are easy to address or answer. I'm happy to answer any questions or chat
> about my suggestions with you on Monday.
Great, thanks.
> === added file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> > @@ -0,0 +1,16 @@
> > +Hello admins,
>
> Maybe "Hello Launchpad administrators," ?
Sure, fixed. Note that these emails only ended up on our internal
error-reports list, but no reason not to make them even nicer :)
> > +--
>
> Note that the signature separator is actually "-- " yep with a trailing space!
> I know that kind of screws the no-trailing-
> have it. ;)
Ah, interesting. Fixed all over the place.
> > +Automatic message from Launchpad.net — https:/
>
> Wow, this is like a failure failure. :)
My mail client linkifies the second, but not the first. I still prefer
the former in an email message, so I made it
"Automatic message from Launchpad.net."
in all the email templates.
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/...
1 | === modified file 'lib/lp/testing/mail_helpers.py' |
2 | --- lib/lp/testing/mail_helpers.py 2009-07-17 00:26:05 +0000 |
3 | +++ lib/lp/testing/mail_helpers.py 2009-08-17 12:22:39 +0000 |
4 | @@ -22,7 +22,7 @@ def pop_notifications(sort_key=None, com |
5 | :param sort_key: define sorting function. sort_key specifies a |
6 | function of one argument that is used to extract a comparison key from |
7 | each list element. (See the sorted() Python built-in.) |
8 | - :param commit: whether to commit before reading email (defauls to False). |
9 | + :param commit: whether to commit before reading email (defaults to True). |
10 | """ |
11 | if commit: |
12 | transaction.commit() |
13 | |
14 | === modified file 'lib/lp/translations/doc/poexport-queue.txt' |
15 | --- lib/lp/translations/doc/poexport-queue.txt 2009-08-14 17:46:32 +0000 |
16 | +++ lib/lp/translations/doc/poexport-queue.txt 2009-08-17 13:15:52 +0000 |
17 | @@ -12,9 +12,8 @@ and the ones that failed with the error |
18 | >>> import transaction |
19 | >>> from zope.component import getUtility |
20 | >>> from canonical.launchpad.interfaces import IPersonSet |
21 | - >>> from lp.services.mail.stub import test_emails |
22 | + >>> from lp.testing.mail_helpers import pop_notifications, print_emails |
23 | >>> from lp.translations.scripts.po_export_queue import ExportResult |
24 | - >>> from lp.translations.tests.helpers import print_mail_subject_and_body |
25 | >>> import logging |
26 | >>> logger = logging.getLogger() |
27 | |
28 | @@ -57,76 +56,73 @@ In this case, it should be None, so the |
29 | >>> result.url = None |
30 | >>> result.notify() |
31 | |
32 | - # We need to commit the transaction to get the emails in the queue. |
33 | - >>> transaction.commit() |
34 | - |
35 | As usual, when there is an error, two emails should be sent: |
36 | |
37 | + >>> test_emails = pop_notifications() |
38 | >>> len(test_emails) |
39 | 2 |
40 | |
41 | - >>> while len(test_emails) > 0: |
42 | - ... from_addrs, to_addrs, body = test_emails.pop() |
43 | - ... if 'carlos@canonical.com' in to_addrs: |
44 | - ... carlos_addrs = to_addrs |
45 | - ... carlos_body = body |
46 | + >>> for email in test_emails: |
47 | + ... if 'carlos@canonical.com' in email['to']: |
48 | + ... carlos_email = email |
49 | ... else: |
50 | - ... admins_addrs = to_addrs |
51 | - ... admins_body = body |
52 | + ... admin_email = email |
53 | |
54 | One is for the user with the error notification. |
55 | |
56 | - >>> print carlos_addrs |
57 | - ['carlos@canonical.com'] |
58 | - >>> print_mail_subject_and_body(carlos_body) |
59 | + >>> print_emails(notifications=[carlos_email]) |
60 | + From: ... |
61 | + To: carlos@canonical.com |
62 | Subject: Launchpad translation download: Evolution trunk - |
63 | evolution-2.2 template |
64 | - > Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
65 | - > |
66 | - > Launchpad encountered problems exporting the files you requested. |
67 | - > The Launchpad Translations team has been notified of this problem. |
68 | - > Please reply to this email for further assistance. |
69 | - > |
70 | - > If you want to retry your request, you can do so at |
71 | - > |
72 | - > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
73 | - > port. |
74 | - > |
75 | - > -- |
76 | - > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
77 | - > |
78 | + Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
79 | + <BLANKLINE> |
80 | + Launchpad encountered problems exporting the files you requested. |
81 | + The Launchpad Translations team has been notified of this problem. |
82 | + Please reply to this email for further assistance. |
83 | + <BLANKLINE> |
84 | + If you want to retry your request, you can do so at |
85 | + <BLANKLINE> |
86 | + http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
87 | + port. |
88 | + <BLANKLINE> |
89 | + -- = |
90 | + Automatic message from Launchpad.net. |
91 | + <BLANKLINE> |
92 | + ---------------------------------------- |
93 | |
94 | And the other to the admins. This one lists the files that were being |
95 | exported as context to help tracking down any bugs. |
96 | |
97 | - >>> print admins_addrs |
98 | - ['launchpad-error-reports@lists.canonical.com'] |
99 | - >>> print_mail_subject_and_body(admins_body) |
100 | + >>> print_emails(notifications=[admin_email]) |
101 | + From: ... |
102 | + To: launchpad-error-reports@lists.canonical.com |
103 | Subject: Launchpad translation download errors: Evolution trunk - |
104 | evolution-2.2 template |
105 | - > Hello admins, |
106 | - > |
107 | - > Launchpad encountered problems exporting translation files |
108 | - > requested by Carlos Perell=C3=B3 Mar=C3=ADn (carlos) at |
109 | - > |
110 | - > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
111 | - > port |
112 | - > |
113 | - > This means we have a bug in Launchpad that needs to be fixed |
114 | - > before this export can proceed. Here is the error we got: |
115 | - > |
116 | - > Traceback (most recent call last): |
117 | + Hello Launchpad administrators, |
118 | + <BLANKLINE> |
119 | + Launchpad encountered problems exporting translation files |
120 | + requested by Carlos Perell=C3=B3 Mar=C3=ADn (carlos) at |
121 | + <BLANKLINE> |
122 | + http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
123 | + port |
124 | + <BLANKLINE> |
125 | + This means we have a bug in Launchpad that needs to be fixed |
126 | + before this export can proceed. Here is the error we got: |
127 | + <BLANKLINE> |
128 | + Traceback (most recent call last): |
129 | ... |
130 | - > AssertionError: It's just an error for testing purposes |
131 | - > |
132 | - > |
133 | - > Failed export request included: |
134 | - > * evolution-2.2 in Evolution trunk |
135 | - > * Spanish (es) translation of evolution-2.2 in Evolution trunk |
136 | - > |
137 | - > -- |
138 | - > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
139 | - > |
140 | + AssertionError: It's just an error for testing purposes |
141 | + <BLANKLINE> |
142 | + <BLANKLINE> |
143 | + Failed export request included: |
144 | + * evolution-2.2 in Evolution trunk |
145 | + * Spanish (es) translation of evolution-2.2 in Evolution trunk |
146 | + <BLANKLINE> |
147 | + -- = |
148 | + Automatic message from Launchpad.net. |
149 | + <BLANKLINE> |
150 | + ---------------------------------------- |
151 | |
152 | As a special case, some error messages are poisoned with non-ASCII characters |
153 | and can't be reported without triggering an error themselves. Those are |
154 | @@ -137,61 +133,67 @@ specially handled and reported. |
155 | ... except AssertionError: |
156 | ... result.addFailure() |
157 | >>> result.notify() |
158 | - >>> transaction.commit() |
159 | + |
160 | + >>> test_emails = pop_notifications() |
161 | >>> len(test_emails) |
162 | 2 |
163 | |
164 | - >>> carlos_body = None |
165 | - >>> admins_body = None |
166 | - >>> while len(test_emails) > 0: |
167 | - ... from_addrs, to_addrs, body = test_emails.pop() |
168 | - ... if 'carlos@canonical.com' in to_addrs: |
169 | - ... carlos_body = body |
170 | + >>> carlos_email = None |
171 | + >>> admins_email = None |
172 | + >>> for email in test_emails: |
173 | + ... if 'carlos@canonical.com' in email['to']: |
174 | + ... carlos_email = email |
175 | ... else: |
176 | - ... admins_body = body |
177 | + ... admin_email = email |
178 | |
179 | The user's notification looks no different from that for an ordinary error. |
180 | |
181 | - >>> print_mail_subject_and_body(carlos_body) |
182 | + >>> print_emails(notifications=[carlos_email]) |
183 | + From: ... |
184 | + To: carlos@canonical.com |
185 | Subject: Launchpad translation download: Evolution trunk - |
186 | evolution-2.2 template |
187 | - > Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
188 | - > |
189 | - > Launchpad encountered problems exporting the files you requested. |
190 | - > The Launchpad Translations team has been notified of this problem. |
191 | - > Please reply to this email for further assistance. |
192 | - > |
193 | - > If you want to retry your request, you can do so at |
194 | - > |
195 | - > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
196 | - > port. |
197 | - > |
198 | - > -- |
199 | - > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
200 | - > |
201 | + Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
202 | + <BLANKLINE> |
203 | + Launchpad encountered problems exporting the files you requested. |
204 | + The Launchpad Translations team has been notified of this problem. |
205 | + Please reply to this email for further assistance. |
206 | + <BLANKLINE> |
207 | + If you want to retry your request, you can do so at |
208 | + <BLANKLINE> |
209 | + http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
210 | + port. |
211 | + <BLANKLINE> |
212 | + -- = |
213 | + Automatic message from Launchpad.net. |
214 | + <BLANKLINE> |
215 | + ---------------------------------------- |
216 | |
217 | The one for the administrators, however, does not include the unprintable |
218 | exception text. |
219 | |
220 | - >>> print_mail_subject_and_body(admins_body) |
221 | + >>> print_emails(notifications=[admin_email]) |
222 | + From: ... |
223 | + To: launchpad-error-reports@lists.canonical.com |
224 | Subject: Launchpad translation download errors: Evolution trunk - |
225 | evolution-2.2 template |
226 | - > Hello admins, |
227 | - > |
228 | - > A UnicodeDecodeError occurred while trying to notify you of a |
229 | - > failure during a translation export requested by Carlos ... |
230 | - > (carlos) at |
231 | - > |
232 | - > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
233 | - > port |
234 | - > |
235 | - > Failed export request included: |
236 | - > * evolution-2.2 in Evolution trunk |
237 | - > * Spanish (es) translation of evolution-2.2 in Evolution trunk |
238 | - > |
239 | - > -- |
240 | - > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
241 | - > |
242 | + Hello Launchpad administrators, |
243 | + <BLANKLINE> |
244 | + A UnicodeDecodeError occurred while trying to notify you of a |
245 | + failure during a translation export requested by Carlos ... |
246 | + (carlos) at |
247 | + <BLANKLINE> |
248 | + http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
249 | + port |
250 | + <BLANKLINE> |
251 | + Failed export request included: |
252 | + * evolution-2.2 in Evolution trunk |
253 | + * Spanish (es) translation of evolution-2.2 in Evolution trunk |
254 | + <BLANKLINE> |
255 | + -- = |
256 | + Automatic message from Launchpad.net. |
257 | + <BLANKLINE> |
258 | + ---------------------------------------- |
259 | |
260 | |
261 | Finally, there is the case when there are no errors at all. This is the usual |
262 | @@ -207,46 +209,40 @@ download the requested files. If we don' |
263 | ... |
264 | AssertionError: On success, an exported URL is expected. |
265 | |
266 | -So let's add it: |
267 | +So let's add it and notify the user: |
268 | |
269 | >>> result.url = 'http://someplace.com/somefile.tar.gz' |
270 | - |
271 | -And notify the user. |
272 | - |
273 | >>> result.notify() |
274 | |
275 | - # We need to commit the transaction to get the email in the queue. |
276 | - >>> transaction.commit() |
277 | - |
278 | In this case, there are no errors, so we should get just a single email |
279 | |
280 | + >>> test_emails = pop_notifications() |
281 | >>> len(test_emails) |
282 | 1 |
283 | |
284 | - >>> from_addrs, to_addrs, body = test_emails.pop() |
285 | - |
286 | - >>> print to_addrs |
287 | - ['carlos@canonical.com'] |
288 | - >>> print_mail_subject_and_body(body) |
289 | + >>> print_emails(notifications=test_emails) |
290 | + From: ... |
291 | + To: carlos@canonical.com |
292 | Subject: Launchpad translation download: Evolution trunk - |
293 | evolution-2.2 template |
294 | - > Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
295 | - > |
296 | - > The translation files you requested from Launchpad are ready for |
297 | - > download from the following location: |
298 | - > |
299 | - > http://someplace.com/somefile.tar.gz |
300 | - > |
301 | - > Note: this link will expire in about 1 week. If you want to |
302 | - > download these translations again, you will have to request |
303 | - > them again at |
304 | - > |
305 | - > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
306 | - > port |
307 | - > |
308 | - > -- |
309 | - > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
310 | - > |
311 | + Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
312 | + <BLANKLINE> |
313 | + The translation files you requested from Launchpad are ready for |
314 | + download from the following location: |
315 | + <BLANKLINE> |
316 | + http://someplace.com/somefile.tar.gz |
317 | + <BLANKLINE> |
318 | + Note: this link will expire in about 1 week. If you want to |
319 | + download these translations again, you will have to request |
320 | + them again at |
321 | + <BLANKLINE> |
322 | + http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
323 | + port |
324 | + <BLANKLINE> |
325 | + -- = |
326 | + Automatic message from Launchpad.net. |
327 | + <BLANKLINE> |
328 | + ---------------------------------------- |
329 | |
330 | |
331 | |
332 | @@ -287,34 +283,35 @@ Once the queue is processed, the queue i |
333 | |
334 | And a confirmation email was sent to carlos, the importer. |
335 | |
336 | + >>> test_emails = pop_notifications() |
337 | >>> len(test_emails) |
338 | 1 |
339 | |
340 | The confirmation email shows no errors at all. |
341 | |
342 | - >>> from_addrs, to_addrs, body = test_emails.pop() |
343 | - >>> print to_addrs |
344 | - ['carlos@canonical.com'] |
345 | - >>> print_mail_subject_and_body(body) |
346 | + >>> print_emails(notifications=test_emails) |
347 | + From: ... |
348 | + To: carlos@canonical.com |
349 | Subject: Launchpad translation download: Evolution trunk - |
350 | evolution-2.2 template |
351 | - > Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
352 | - > |
353 | - > The translation files you requested from Launchpad are ready for |
354 | - > download from the following location: |
355 | - > |
356 | - > http://localhost:58000/.../po_evolution-2.2.pot |
357 | - > |
358 | - > Note: this link will expire in about 1 week. If you want to |
359 | - > download these translations again, you will have to request |
360 | - > them again at |
361 | - > |
362 | - > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
363 | - > port |
364 | - > |
365 | - > -- |
366 | - > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
367 | - > |
368 | + Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
369 | + <BLANKLINE> |
370 | + The translation files you requested from Launchpad are ready for |
371 | + download from the following location: |
372 | + <BLANKLINE> |
373 | + http://localhost:58000/.../po_evolution-2.2.pot |
374 | + <BLANKLINE> |
375 | + Note: this link will expire in about 1 week. If you want to |
376 | + download these translations again, you will have to request |
377 | + them again at |
378 | + <BLANKLINE> |
379 | + http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
380 | + port |
381 | + <BLANKLINE> |
382 | + -- = |
383 | + Automatic message from Launchpad.net. |
384 | + <BLANKLINE> |
385 | + ---------------------------------------- |
386 | |
387 | Let's have a closer look at what is being exported. Usually all messages are |
388 | exported but not all messages are equal. Some messages have been imported |
389 | @@ -372,12 +369,11 @@ after having been imported. |
390 | |
391 | Two more email notifications were sent, we'd better get rid of them. |
392 | |
393 | - >>> email = test_emails.pop() |
394 | - >>> email = test_emails.pop() |
395 | + >>> discard = pop_notifications() |
396 | |
397 | Finally, if we try to do an export with an empty queue, we don't do |
398 | anything: |
399 | |
400 | >>> process_queue(transaction, logging.getLogger()) |
401 | - >>> len(test_emails) |
402 | + >>> len(pop_notifications()) |
403 | 0 |
404 | |
405 | === modified file 'lib/lp/translations/doc/poexport-request-productseries.txt' |
406 | --- lib/lp/translations/doc/poexport-request-productseries.txt 2009-08-14 17:46:32 +0000 |
407 | +++ lib/lp/translations/doc/poexport-request-productseries.txt 2009-08-17 13:39:30 +0000 |
408 | @@ -49,35 +49,31 @@ Now we request that the queue be process |
409 | Evolution trunk |
410 | log> Stored file at http://.../launchpad-export.tar.gz |
411 | |
412 | -We need to make sure the Librarian data has settled into place. |
413 | - |
414 | - >>> import transaction |
415 | - >>> transaction.commit() |
416 | - |
417 | The user receives a confirmation email. |
418 | |
419 | - >>> from lp.translations.tests.helpers import print_mail_subject_and_body |
420 | - >>> from lp.services.mail.stub import test_emails |
421 | + >>> from lp.testing.mail_helpers import pop_notifications, print_emails |
422 | + >>> test_emails = pop_notifications() |
423 | >>> len(test_emails) |
424 | 1 |
425 | - >>> from_addr, to_addrs, body = test_emails.pop() |
426 | - >>> print_mail_subject_and_body(body) |
427 | + >>> print_emails(notifications=test_emails) |
428 | + From: ... |
429 | Subject: Launchpad translation download: Evolution trunk |
430 | - > Hello ..., |
431 | - > |
432 | - > The translation files you requested from Launchpad are ready for |
433 | - > download from the following location: |
434 | - > |
435 | - > http://localhost:58000/.../launchpad-export.tar.gz |
436 | - > |
437 | - > Note: this link will expire in about 1 week. If you want to |
438 | - > download these translations again, you will have to request |
439 | - > them again at |
440 | - > |
441 | - > http://translations.launchpad.dev/evolution/trunk/+export |
442 | - > |
443 | - > -- |
444 | - > Automatic message from Launchpad.net... |
445 | + Hello ..., |
446 | + <BLANKLINE> |
447 | + The translation files you requested from Launchpad are ready for |
448 | + download from the following location: |
449 | + <BLANKLINE> |
450 | + http://localhost:58000/.../launchpad-export.tar.gz |
451 | + <BLANKLINE> |
452 | + Note: this link will expire in about 1 week. If you want to |
453 | + download these translations again, you will have to request |
454 | + them again at |
455 | + <BLANKLINE> |
456 | + http://translations.launchpad.dev/evolution/trunk/+export |
457 | + <BLANKLINE> |
458 | + -- = |
459 | + Automatic message from Launchpad.net. |
460 | + ---------------------------------------- |
461 | |
462 | The email contains a URL linking to where the exported file can be downloaded. |
463 | |
464 | @@ -87,6 +83,7 @@ The email contains a URL linking to wher |
465 | ... urls = re.compile(r'^ *(http://.*)$', re.M).findall(text) |
466 | ... return urls[0] |
467 | |
468 | + >>> body = test_emails[0].get_payload() |
469 | >>> url = extract_url(body) |
470 | |
471 | Let's download it and make sure the contents look ok. |
472 | |
473 | === modified file 'lib/lp/translations/doc/poexport-request.txt' |
474 | --- lib/lp/translations/doc/poexport-request.txt 2009-08-14 17:46:32 +0000 |
475 | +++ lib/lp/translations/doc/poexport-request.txt 2009-08-17 13:13:13 +0000 |
476 | @@ -8,6 +8,7 @@ serve those requests properly. |
477 | ... IPOExportRequestSet) |
478 | >>> from lp.translations.utilities.tests.helpers import ( |
479 | ... is_valid_mofile) |
480 | + >>> from lp.testing.mail_helpers import pop_notifications, print_emails |
481 | |
482 | This is a dummy logger class to capture the export's log messages. |
483 | |
484 | @@ -33,9 +34,10 @@ This is a dummy logger class to capture |
485 | |
486 | Here's somebody to make a request. |
487 | |
488 | - >>> from lp.registry.model.person import Person |
489 | - >>> person = Person.get(1) |
490 | - |
491 | + >>> person = factory.makePerson( |
492 | + ... email='downloader@example.com', |
493 | + ... name='downloader', |
494 | + ... displayname='Happy Downloader') |
495 | |
496 | == Requesting PO files == |
497 | |
498 | @@ -57,42 +59,36 @@ Now we request that the queue be process |
499 | |
500 | >>> from lp.translations.scripts.po_export_queue import process_queue |
501 | >>> process_queue(MockTransactionManager(), MockLogger()) |
502 | - log> Exporting objects for Mark Shuttleworth, related to template pmount |
503 | + log> Exporting objects for Happy Downloader, related to template pmount |
504 | in Ubuntu Hoary package "pmount" |
505 | log> Stored file at http://.../launchpad-export.tar.gz |
506 | |
507 | -We need to make sure the Librarian data has settled into place. |
508 | - |
509 | - >>> import transaction |
510 | - >>> transaction.commit() |
511 | - |
512 | The user receives a confirmation email. |
513 | |
514 | - >>> from lp.translations.tests.helpers import print_mail_subject_and_body |
515 | - >>> from lp.services.mail.stub import test_emails |
516 | - >>> len(test_emails) |
517 | + >>> emails = pop_notifications() |
518 | + >>> len(emails) |
519 | 1 |
520 | - >>> from_addr, to_addrs, body = test_emails.pop() |
521 | - >>> to_addrs |
522 | - ['mark@hbd.com'] |
523 | - >>> print_mail_subject_and_body(body) |
524 | + >>> print_emails(notifications=emails) |
525 | + From: ... |
526 | + To: downloader@example.com |
527 | Subject: Launchpad translation download: Ubuntu Hoary pmount - |
528 | pmount template |
529 | - > Hello Mark Shuttleworth, |
530 | - > |
531 | - > The translation files you requested from Launchpad are ready for |
532 | - > download from the following location: |
533 | - > |
534 | - > http://localhost:58000/.../launchpad-export.tar.gz |
535 | - > |
536 | - > Note: this link will expire in about 1 week. If you want to |
537 | - > download these translations again, you will have to request |
538 | - > them again at |
539 | - > |
540 | - > http://translations.launchpad.../hoary/+source/pmount/+pots/p... |
541 | - > |
542 | - > -- |
543 | - > Automatic message from Launchpad.net... |
544 | + Hello Happy Downloader, |
545 | + <BLANKLINE> |
546 | + The translation files you requested from Launchpad are ready for |
547 | + download from the following location: |
548 | + <BLANKLINE> |
549 | + http://localhost:58000/.../launchpad-export.tar.gz |
550 | + <BLANKLINE> |
551 | + Note: this link will expire in about 1 week. If you want to |
552 | + download these translations again, you will have to request |
553 | + them again at |
554 | + <BLANKLINE> |
555 | + http://translations.launchpad.../hoary/+source/pmount/+pots/p... |
556 | + <BLANKLINE> |
557 | + -- = |
558 | + Automatic message from Launchpad.net. |
559 | + ---------------------------------------- |
560 | |
561 | The email contains a URL linking to where the exported file can be downloaded. |
562 | |
563 | @@ -102,6 +98,7 @@ The email contains a URL linking to wher |
564 | ... urls = re.compile(r'^ *(http://.*)$', re.M).findall(text) |
565 | ... return urls[0] |
566 | |
567 | + >>> body = emails.pop().get_payload() |
568 | >>> url = extract_url(body) |
569 | |
570 | Let's download it and make sure the contents look ok. |
571 | @@ -192,41 +189,40 @@ Let's try requesting an export in MO for |
572 | ... TranslationFileFormat) |
573 | >>> request_set.addRequest(person, None, [cs], TranslationFileFormat.MO) |
574 | >>> process_queue(MockTransactionManager(), MockLogger()) |
575 | - log> Exporting objects for Mark Shuttleworth, related to template pmount |
576 | + log> Exporting objects for Happy Downloader, related to template pmount |
577 | in Ubuntu Hoary package "pmount" |
578 | log> Stored file at http://.../cs_LC_MESSAGES_pmount.mo |
579 | |
580 | - >>> transaction.commit() |
581 | - |
582 | - >>> len(test_emails) |
583 | + >>> emails = pop_notifications() |
584 | + >>> len(emails) |
585 | 1 |
586 | - >>> from_addr, to_addrs, body = test_emails.pop() |
587 | - >>> to_addrs |
588 | - ['mark@hbd.com'] |
589 | - >>> print_mail_subject_and_body(body) |
590 | + >>> print_emails(notifications=emails) |
591 | + From: ... |
592 | + To: downloader@example.com |
593 | Subject: Launchpad translation download: Ubuntu Hoary pmount - Czech |
594 | translation of pmount |
595 | - > Hello Mark Shuttleworth, |
596 | - > |
597 | - > The translation files you requested from Launchpad are ready for |
598 | - > download from the following location: |
599 | - > |
600 | - > http://localhost:58000/.../cs_LC_MESSAGES_pmount.mo |
601 | - > |
602 | - > Note: this link will expire in about 1 week. If you want to |
603 | - > download these translations again, you will have to request |
604 | - > them again at |
605 | - > |
606 | - > http://translations.launchpad.../pmount/+pots/pmoun= |
607 | - > t/cs/+export |
608 | - > |
609 | - > -- |
610 | - > Automatic message from Launchpad.net... |
611 | - |
612 | - >>> url = extract_url(body) |
613 | + Hello Happy Downloader, |
614 | + <BLANKLINE> |
615 | + The translation files you requested from Launchpad are ready for |
616 | + download from the following location: |
617 | + <BLANKLINE> |
618 | + http://localhost:58000/.../cs_LC_MESSAGES_pmount.mo |
619 | + <BLANKLINE> |
620 | + Note: this link will expire in about 1 week. If you want to |
621 | + download these translations again, you will have to request |
622 | + them again at |
623 | + <BLANKLINE> |
624 | + http://translations.launchpad.../pmount/+pots/pmoun= |
625 | + t/cs/+export |
626 | + <BLANKLINE> |
627 | + -- = |
628 | + Automatic message from Launchpad.net. |
629 | + ---------------------------------------- |
630 | |
631 | Check whether we generated a good .mo file. |
632 | |
633 | + >>> body = emails.pop().get_payload() |
634 | + >>> url = extract_url(body) |
635 | >>> is_valid_mofile(urllib2.urlopen(url).read()) |
636 | True |
637 | |
638 | @@ -269,13 +265,13 @@ The script is run. |
639 | >>> (output, empty) = process.communicate() |
640 | >>> print output |
641 | INFO creating lockfile |
642 | - DEBUG Exporting objects for Mark Shuttleworth, related to template |
643 | + DEBUG Exporting objects for Happy Downloader, related to template |
644 | evolution-2.2 in Evolution trunk |
645 | - DEBUG Exporting objects for Mark Shuttleworth, related to template |
646 | + DEBUG Exporting objects for Happy Downloader, related to template |
647 | pmount in Ubuntu Hoary package "pmount" |
648 | - DEBUG Exporting objects for Mark Shuttleworth, related to template |
649 | + DEBUG Exporting objects for Happy Downloader, related to template |
650 | netapplet in NetApplet trunk |
651 | - DEBUG Exporting objects for Mark Shuttleworth, related to template |
652 | + DEBUG Exporting objects for Happy Downloader, related to template |
653 | alsa-utils in alsa-utils trunk |
654 | INFO Stored file at http://.../launchpad-export.tar.gz |
655 | DEBUG Removing lock file: /var/lock/launchpad-rosetta-export-queue.lock |
656 | |
657 | === modified file 'lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt' |
658 | --- lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt 2009-08-14 16:33:57 +0000 |
659 | +++ lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt 2009-08-17 13:16:32 +0000 |
660 | @@ -1,4 +1,4 @@ |
661 | -Hello admins, |
662 | +Hello Launchpad administrators, |
663 | |
664 | Launchpad encountered problems exporting translation files |
665 | requested by %(person)s (%(person_id)s) at |
666 | @@ -12,5 +12,5 @@ before this export can proceed. Here is |
667 | |
668 | %(failed_requests)s |
669 | |
670 | --- |
671 | -Automatic message from Launchpad.net — https://launchpad.net/ |
672 | +-- |
673 | +Automatic message from Launchpad.net. |
674 | |
675 | === modified file 'lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt' |
676 | --- lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt 2009-08-14 16:33:57 +0000 |
677 | +++ lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt 2009-08-17 13:16:39 +0000 |
678 | @@ -1,4 +1,4 @@ |
679 | -Hello admins, |
680 | +Hello Launchpad administrators, |
681 | |
682 | A UnicodeDecodeError occurred while trying to notify you of a |
683 | failure during a translation export requested by %(person)s |
684 | @@ -8,5 +8,5 @@ failure during a translation export requ |
685 | |
686 | %(failed_requests)s |
687 | |
688 | --- |
689 | -Automatic message from Launchpad.net — https://launchpad.net/ |
690 | +-- |
691 | +Automatic message from Launchpad.net. |
692 | |
693 | === modified file 'lib/lp/translations/emailtemplates/poexport-failure.txt' |
694 | --- lib/lp/translations/emailtemplates/poexport-failure.txt 2009-08-14 16:33:57 +0000 |
695 | +++ lib/lp/translations/emailtemplates/poexport-failure.txt 2009-08-17 12:51:36 +0000 |
696 | @@ -8,5 +8,5 @@ If you want to retry your request, you c |
697 | |
698 | %(request_url)s. |
699 | |
700 | --- |
701 | -Automatic message from Launchpad.net — https://launchpad.net/ |
702 | +-- |
703 | +Automatic message from Launchpad.net. |
704 | |
705 | === modified file 'lib/lp/translations/emailtemplates/poexport-success.txt' |
706 | --- lib/lp/translations/emailtemplates/poexport-success.txt 2009-08-14 16:33:57 +0000 |
707 | +++ lib/lp/translations/emailtemplates/poexport-success.txt 2009-08-17 12:51:44 +0000 |
708 | @@ -11,5 +11,5 @@ them again at |
709 | |
710 | %(request_url)s |
711 | |
712 | --- |
713 | -Automatic message from Launchpad.net — https://launchpad.net/ |
714 | +-- |
715 | +Automatic message from Launchpad.net. |
716 | |
717 | === modified file 'lib/lp/translations/scripts/po_export_queue.py' |
718 | --- lib/lp/translations/scripts/po_export_queue.py 2009-08-14 18:02:09 +0000 |
719 | +++ lib/lp/translations/scripts/po_export_queue.py 2009-08-17 13:36:26 +0000 |
720 | @@ -38,7 +38,7 @@ class ExportResult: |
721 | |
722 | This class has three main attributes: |
723 | |
724 | - - person: A person requesting this export. |
725 | + - person: The person requesting this export. |
726 | - url: The Librarian URL for any successfully exported files. |
727 | - failure: Failure gotten while exporting. |
728 | """ |
729 | @@ -138,7 +138,9 @@ class ExportResult: |
730 | potemplate.sourcepackagename) |
731 | sourcepackages.add(sourcepackage) |
732 | else: |
733 | - pass |
734 | + raise AssertionError( |
735 | + "Requesting a translation export which belongs to " |
736 | + "neither a ProductSeries nor a SourcePackage.") |
737 | |
738 | if len(pofiles) == 1 and len(direct_potemplates) == 0: |
739 | # One POFile was requested. |
740 | @@ -162,18 +164,21 @@ class ExportResult: |
741 | |
742 | if IPOTemplate.providedBy(export_requested_at): |
743 | if len(sourcepackages) > 0: |
744 | - sp = sourcepackages.pop() |
745 | - if sp.getCurrentTranslationTemplates().count() == 1: |
746 | - export_requested_at = sp |
747 | + container = sourcepackages.pop() |
748 | elif len(productseries) > 0: |
749 | - ps = productseries.pop() |
750 | - if ps.getCurrentTranslationTemplates().count() == 1: |
751 | - export_requested_at = ps |
752 | + container = productseries.pop() |
753 | + else: |
754 | + raise AssertionError( |
755 | + "Requesting a translation export which belongs to " |
756 | + "neither a ProductSeries nor a SourcePackage.") |
757 | + if container.getCurrentTranslationTemplates().count() == 1: |
758 | + export_requested_at = container |
759 | |
760 | return export_requested_at |
761 | |
762 | |
763 | def _getRequestedExportsNames(self): |
764 | + """Return a list of display names for requested exports.""" |
765 | requested_names = [] |
766 | for translation_object in self.requested_exports: |
767 | if IPOTemplate.providedBy(translation_object): |
768 | @@ -187,7 +192,7 @@ class ExportResult: |
769 | def _getFailureEmailBody(self): |
770 | """Send an email notification about the export failing.""" |
771 | template = helpers.get_email_template( |
772 | - 'poexport-failure.txt', 'translations').decode('utf-8') |
773 | + 'poexport-failure.txt', 'translations') |
774 | return template % { |
775 | 'person' : self.person.displayname, |
776 | 'request_url' : self.request_url, |
777 | @@ -199,7 +204,7 @@ class ExportResult: |
778 | if len(failed_requests) > 0: |
779 | failed_requests_text = 'Failed export request included:\n' |
780 | failed_requests_text += '\n'.join( |
781 | - [' * ' + request for request in failed_requests]) |
782 | + ' * ' + request for request in failed_requests) |
783 | else: |
784 | failed_requests_text = 'There were no export requests.' |
785 | return failed_requests_text |
786 | @@ -208,7 +213,7 @@ class ExportResult: |
787 | """Send an email notification about failed export to admins.""" |
788 | template = helpers.get_email_template( |
789 | 'poexport-failure-admin-notification.txt', |
790 | - 'translations').decode('utf-8') |
791 | + 'translations') |
792 | failed_requests = self._getFailedRequestsDescription() |
793 | return template % { |
794 | 'person' : self.person.displayname, |
795 | @@ -222,7 +227,7 @@ class ExportResult: |
796 | """Send an email notification to admins about UnicodeDecodeError.""" |
797 | template = helpers.get_email_template( |
798 | 'poexport-failure-unicodedecodeerror.txt', |
799 | - 'translations').decode('utf-8') |
800 | + 'translations') |
801 | failed_requests = self._getFailedRequestsDescription() |
802 | return template % { |
803 | 'person' : self.person.displayname, |
804 | @@ -234,7 +239,7 @@ class ExportResult: |
805 | def _getSuccessEmailBody(self): |
806 | """Send an email notification about the export working.""" |
807 | template = helpers.get_email_template( |
808 | - 'poexport-success.txt', 'translations').decode('utf-8') |
809 | + 'poexport-success.txt', 'translations') |
810 | return template % { |
811 | 'person' : self.person.displayname, |
812 | 'download_url' : self.url, |
813 | |
814 | === removed file 'lib/lp/translations/tests/helpers.py' |
815 | --- lib/lp/translations/tests/helpers.py 2009-08-14 17:46:32 +0000 |
816 | +++ lib/lp/translations/tests/helpers.py 1970-01-01 00:00:00 +0000 |
817 | @@ -1,19 +0,0 @@ |
818 | -# Copyright 2009 Canonical Ltd. This software is licensed under the |
819 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
820 | - |
821 | -"""Helper module reused in different tests.""" |
822 | - |
823 | -__metaclass__ = type |
824 | - |
825 | -__all__ = [ |
826 | - 'print_mail_subject_and_body' |
827 | - ] |
828 | - |
829 | -import email |
830 | - |
831 | -def print_mail_subject_and_body(contents): |
832 | - msg = email.message_from_string(contents) |
833 | - body = msg.get_payload() |
834 | - print 'Subject: %s' % (msg['subject']) |
835 | - for line in body.split('\n'): |
836 | - print ">", line |
837 |
Barry Warsaw (barry) wrote : | # |
On Aug 17, 2009, at 01:48 PM, Данило Шеган wrote:
>I've applied most of your recommendations. The ones I didn't were about
>'positive tests' (i.e. is 'is not None' really a negative test?) and I
>haven't provided unit tests. Still, I've modified doctests to use
>mail_helpers, so that resulted in a nice big incremental diff.
Indeed, that looks fantastic. Thanks!
>У пет, 14. 08 2009. у 16:05 -0400, Barry Warsaw пише:
>> > * doc/poexport-
>> > doc/poexport-
>> > doc/poexport-
>> > There are a lot of changes to tests that test how emails are generated:
>> > basically, half of the branch is there (440 lines). The changes there
>> > include introducing a shared helper print_mail_
>> > print subject along with body of the email, and changes to test new
>> > email contents and subjects. Some minor changes to how ExportResult is
>> > constructed were necessary as well to conform with the new API (i.e. pass
>> > person, requested exports and logger).
>>
>> Do you know about lp.testing.
>> that plus ellipses to cut down on the output. Is there a reason why you'd
>> want print_mail_
>
>Yes, I've looked through existing helpers. I've already struggled to
>keep the diff size reasonable, and this would have made it only worse.
>Though, I am happy to migrate existing tests to the use them, and I did
>so. Note that only this change means 640 lines of diff in the
>incremental diff ;)
That's the way to do it! :) Really, it looks great now.
>> === added file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
>> > @@ -0,0 +1,16 @@
>> > +Hello admins,
>>
>> Maybe "Hello Launchpad administrators," ?
>
>Sure, fixed. Note that these emails only ended up on our internal
>error-reports list, but no reason not to make them even nicer :)
If we can't be nice to ourselves, we can't be nice to our users :)
>> > +Automatic message from Launchpad.net — https:/
>>
>> Wow, this is like a failure failure. :)
>
>My mail client linkifies the second, but not the first. I still prefer
>the former in an email message, so I made it
>
> "Automatic message from Launchpad.net."
>
>in all the email templates.
Y'know, I didn't really notice that that was an em-dash. I thought it was a
plain old '-' but I guess that was just my editor's font playing tricks on
me. In any case, I think keeping it pure ASCII is a good thing (as you saw by
removing all those utf-8 conversions).
But I wasn't actually complaining about it, I was just observing that the
message appeared to be sent when a failure message failed. Still, I'm glad my
Friday-addled brain was able to reach through the fog and offer a not-horrible
suggestion. ;)
>> Is it really more convenient to access the logger through the instance rather
>> than use the module global? I can almost promise you...
Preview Diff
1 | === modified file 'lib/lp/translations/doc/poexport-queue.txt' |
2 | --- lib/lp/translations/doc/poexport-queue.txt 2009-07-02 17:16:50 +0000 |
3 | +++ lib/lp/translations/doc/poexport-queue.txt 2009-08-14 17:46:32 +0000 |
4 | @@ -9,17 +9,14 @@ |
5 | ExportResult class is used to control the list of exported files that succeed |
6 | and the ones that failed with the error associated. |
7 | |
8 | - >>> import transaction |
9 | - >>> from zope.component import getUtility |
10 | - >>> from canonical.launchpad.interfaces import IPersonSet |
11 | - >>> from lp.services.mail.stub import test_emails |
12 | - >>> from lp.translations.scripts.po_export_queue import ExportResult |
13 | - |
14 | - # Function to print mail content without headers. |
15 | - >>> def print_mail_body(body): |
16 | - ... lines = body[body.index('\n\n')+2:].split('\n') |
17 | - ... for line in lines: |
18 | - ... print ">", line |
19 | + >>> import transaction |
20 | + >>> from zope.component import getUtility |
21 | + >>> from canonical.launchpad.interfaces import IPersonSet |
22 | + >>> from lp.services.mail.stub import test_emails |
23 | + >>> from lp.translations.scripts.po_export_queue import ExportResult |
24 | + >>> from lp.translations.tests.helpers import print_mail_subject_and_body |
25 | + >>> import logging |
26 | + >>> logger = logging.getLogger() |
27 | |
28 | When there is an error, the system will notify it. |
29 | |
30 | @@ -29,10 +26,10 @@ |
31 | >>> from lp.translations.model.potemplate import POTemplate |
32 | >>> potemplate = POTemplate.get(1) |
33 | >>> pofile = potemplate.getPOFileByLang('es') |
34 | + >>> personset = getUtility(IPersonSet) |
35 | + >>> carlos = personset.getByName('carlos') |
36 | |
37 | - >>> result = ExportResult('testing export') |
38 | - >>> result.object_names.append(potemplate.displayname) |
39 | - >>> result.object_names.append(pofile.title) |
40 | + >>> result = ExportResult(carlos, [potemplate, pofile], logger) |
41 | |
42 | Record the error. |
43 | |
44 | @@ -50,9 +47,7 @@ |
45 | given that we set a URL, the notification will detect the programming error. |
46 | In this example, 'carlos' will be the one that did the request. |
47 | |
48 | - >>> personset = getUtility(IPersonSet) |
49 | - >>> carlos = personset.getByName('carlos') |
50 | - >>> result.notify(carlos) |
51 | + >>> result.notify() |
52 | Traceback (most recent call last): |
53 | ... |
54 | AssertionError: We cannot have a URL for the export and a failure. |
55 | @@ -60,15 +55,15 @@ |
56 | In this case, it should be None, so the notify works. |
57 | |
58 | >>> result.url = None |
59 | - >>> result.notify(carlos) |
60 | + >>> result.notify() |
61 | |
62 | # We need to commit the transaction to get the emails in the queue. |
63 | >>> transaction.commit() |
64 | |
65 | As usual, when there is an error, two emails should be sent: |
66 | |
67 | - >>> len(test_emails) == 2 |
68 | - True |
69 | + >>> len(test_emails) |
70 | + 2 |
71 | |
72 | >>> while len(test_emails) > 0: |
73 | ... from_addrs, to_addrs, body = test_emails.pop() |
74 | @@ -83,38 +78,55 @@ |
75 | |
76 | >>> print carlos_addrs |
77 | ['carlos@canonical.com'] |
78 | - >>> print_mail_body(carlos_body) |
79 | - > |
80 | + >>> print_mail_subject_and_body(carlos_body) |
81 | + Subject: Launchpad translation download: Evolution trunk - |
82 | + evolution-2.2 template |
83 | > Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
84 | - > |
85 | + > |
86 | > Launchpad encountered problems exporting the files you requested. |
87 | > The Launchpad Translations team has been notified of this problem. |
88 | > Please reply to this email for further assistance. |
89 | > |
90 | + > If you want to retry your request, you can do so at |
91 | + > |
92 | + > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
93 | + > port. |
94 | + > |
95 | + > -- |
96 | + > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
97 | + > |
98 | |
99 | And the other to the admins. This one lists the files that were being |
100 | exported as context to help tracking down any bugs. |
101 | |
102 | >>> print admins_addrs |
103 | ['launchpad-error-reports@lists.canonical.com'] |
104 | - >>> print_mail_body(admins_body) |
105 | - > |
106 | + >>> print_mail_subject_and_body(admins_body) |
107 | + Subject: Launchpad translation download errors: Evolution trunk - |
108 | + evolution-2.2 template |
109 | > Hello admins, |
110 | - > |
111 | + > |
112 | > Launchpad encountered problems exporting translation files |
113 | - > requested by Carlos Perell=C3=B3 Mar=C3=ADn. |
114 | + > requested by Carlos Perell=C3=B3 Mar=C3=ADn (carlos) at |
115 | + > |
116 | + > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
117 | + > port |
118 | > |
119 | > This means we have a bug in Launchpad that needs to be fixed |
120 | - > before this export can proceed. Here is the list of failed |
121 | - > files and the error we got: |
122 | - > |
123 | + > before this export can proceed. Here is the error we got: |
124 | + > |
125 | > Traceback (most recent call last): |
126 | ... |
127 | > AssertionError: It's just an error for testing purposes |
128 | > |
129 | - > The failed request involved these objects: |
130 | - > evolution-2.2 in Evolution trunk |
131 | - > Spanish (es) translation of evolution-2.2 in Evolution trunk |
132 | + > |
133 | + > Failed export request included: |
134 | + > * evolution-2.2 in Evolution trunk |
135 | + > * Spanish (es) translation of evolution-2.2 in Evolution trunk |
136 | + > |
137 | + > -- |
138 | + > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
139 | + > |
140 | |
141 | As a special case, some error messages are poisoned with non-ASCII characters |
142 | and can't be reported without triggering an error themselves. Those are |
143 | @@ -124,10 +136,10 @@ |
144 | ... raise AssertionError, "Really nasty \xc3 non-ASCII error!" |
145 | ... except AssertionError: |
146 | ... result.addFailure() |
147 | - >>> result.notify(carlos) |
148 | + >>> result.notify() |
149 | >>> transaction.commit() |
150 | - >>> len(test_emails) == 2 |
151 | - True |
152 | + >>> len(test_emails) |
153 | + 2 |
154 | |
155 | >>> carlos_body = None |
156 | >>> admins_body = None |
157 | @@ -140,39 +152,57 @@ |
158 | |
159 | The user's notification looks no different from that for an ordinary error. |
160 | |
161 | - >>> print_mail_body(carlos_body) |
162 | - > |
163 | + >>> print_mail_subject_and_body(carlos_body) |
164 | + Subject: Launchpad translation download: Evolution trunk - |
165 | + evolution-2.2 template |
166 | > Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
167 | - > |
168 | + > |
169 | > Launchpad encountered problems exporting the files you requested. |
170 | > The Launchpad Translations team has been notified of this problem. |
171 | > Please reply to this email for further assistance. |
172 | > |
173 | + > If you want to retry your request, you can do so at |
174 | + > |
175 | + > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
176 | + > port. |
177 | + > |
178 | + > -- |
179 | + > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
180 | + > |
181 | |
182 | The one for the administrators, however, does not include the unprintable |
183 | exception text. |
184 | |
185 | - >>> print_mail_body(admins_body) |
186 | - > |
187 | + >>> print_mail_subject_and_body(admins_body) |
188 | + Subject: Launchpad translation download errors: Evolution trunk - |
189 | + evolution-2.2 template |
190 | > Hello admins, |
191 | - > |
192 | + > |
193 | > A UnicodeDecodeError occurred while trying to notify you of a |
194 | > failure during a translation export requested by Carlos ... |
195 | - > |
196 | - > The failed request involved these objects: |
197 | - > evolution-2.2 in Evolution trunk |
198 | - > Spanish (es) translation of evolution-2.2 in Evolution trunk |
199 | + > (carlos) at |
200 | + > |
201 | + > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
202 | + > port |
203 | + > |
204 | + > Failed export request included: |
205 | + > * evolution-2.2 in Evolution trunk |
206 | + > * Spanish (es) translation of evolution-2.2 in Evolution trunk |
207 | + > |
208 | + > -- |
209 | + > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
210 | + > |
211 | |
212 | |
213 | Finally, there is the case when there are no errors at all. This is the usual |
214 | case. |
215 | |
216 | - >>> result = ExportResult('testing export') |
217 | + >>> result = ExportResult(carlos, [potemplate, pofile], logger) |
218 | |
219 | As noted before, result.url should be set to the URL where the user can |
220 | download the requested files. If we don't set it, the export will fail: |
221 | |
222 | - >>> result.notify(carlos) |
223 | + >>> result.notify() |
224 | Traceback (most recent call last): |
225 | ... |
226 | AssertionError: On success, an exported URL is expected. |
227 | @@ -183,28 +213,41 @@ |
228 | |
229 | And notify the user. |
230 | |
231 | - >>> result.notify(carlos) |
232 | + >>> result.notify() |
233 | |
234 | # We need to commit the transaction to get the email in the queue. |
235 | >>> transaction.commit() |
236 | |
237 | In this case, there are no errors, so we should get just a single email |
238 | |
239 | - >>> len(test_emails) == 1 |
240 | - True |
241 | + >>> len(test_emails) |
242 | + 1 |
243 | |
244 | >>> from_addrs, to_addrs, body = test_emails.pop() |
245 | |
246 | >>> print to_addrs |
247 | ['carlos@canonical.com'] |
248 | - >>> print_mail_body(body) |
249 | - > |
250 | + >>> print_mail_subject_and_body(body) |
251 | + Subject: Launchpad translation download: Evolution trunk - |
252 | + evolution-2.2 template |
253 | > Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
254 | - > |
255 | + > |
256 | > The translation files you requested from Launchpad are ready for |
257 | > download from the following location: |
258 | - > |
259 | - > http://someplace.com/somefile.tar.gz |
260 | + > |
261 | + > http://someplace.com/somefile.tar.gz |
262 | + > |
263 | + > Note: this link will expire in about 1 week. If you want to |
264 | + > download these translations again, you will have to request |
265 | + > them again at |
266 | + > |
267 | + > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
268 | + > port |
269 | + > |
270 | + > -- |
271 | + > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
272 | + > |
273 | + |
274 | |
275 | |
276 | == process_queue() == |
277 | @@ -236,7 +279,6 @@ |
278 | |
279 | Once the queue is processed, the queue is empty again. |
280 | |
281 | - >>> import logging |
282 | >>> process_queue(transaction, logging.getLogger()) |
283 | INFO:...Stored file at http://.../po_evolution-2.2.pot |
284 | |
285 | @@ -253,14 +295,26 @@ |
286 | >>> from_addrs, to_addrs, body = test_emails.pop() |
287 | >>> print to_addrs |
288 | ['carlos@canonical.com'] |
289 | - >>> print_mail_body(body) |
290 | - > |
291 | + >>> print_mail_subject_and_body(body) |
292 | + Subject: Launchpad translation download: Evolution trunk - |
293 | + evolution-2.2 template |
294 | > Hello Carlos Perell=C3=B3 Mar=C3=ADn, |
295 | - > |
296 | + > |
297 | > The translation files you requested from Launchpad are ready for |
298 | > download from the following location: |
299 | - > |
300 | + > |
301 | > http://localhost:58000/.../po_evolution-2.2.pot |
302 | + > |
303 | + > Note: this link will expire in about 1 week. If you want to |
304 | + > download these translations again, you will have to request |
305 | + > them again at |
306 | + > |
307 | + > http://translations.launchpad.../trunk/+pots/evolution-2.2/+ex= |
308 | + > port |
309 | + > |
310 | + > -- |
311 | + > Automatic message from Launchpad.net =E2=80=94 https://launchpad.net/ |
312 | + > |
313 | |
314 | Let's have a closer look at what is being exported. Usually all messages are |
315 | exported but not all messages are equal. Some messages have been imported |
316 | |
317 | === modified file 'lib/lp/translations/doc/poexport-request-productseries.txt' |
318 | --- lib/lp/translations/doc/poexport-request-productseries.txt 2009-07-23 17:49:31 +0000 |
319 | +++ lib/lp/translations/doc/poexport-request-productseries.txt 2009-08-14 17:46:32 +0000 |
320 | @@ -56,23 +56,28 @@ |
321 | |
322 | The user receives a confirmation email. |
323 | |
324 | - >>> def print_mail_body(body): |
325 | - ... lines = body[body.index('\n\n')+2:].split('\n') |
326 | - ... for line in lines: |
327 | - ... print ">", line |
328 | - |
329 | + >>> from lp.translations.tests.helpers import print_mail_subject_and_body |
330 | >>> from lp.services.mail.stub import test_emails |
331 | >>> len(test_emails) |
332 | 1 |
333 | >>> from_addr, to_addrs, body = test_emails.pop() |
334 | - >>> print_mail_body(body) |
335 | - > |
336 | + >>> print_mail_subject_and_body(body) |
337 | + Subject: Launchpad translation download: Evolution trunk |
338 | > Hello ..., |
339 | > |
340 | > The translation files you requested from Launchpad are ready for |
341 | > download from the following location: |
342 | > |
343 | - > http://localhost:58000/.../launchpad-export.tar.gz |
344 | + > http://localhost:58000/.../launchpad-export.tar.gz |
345 | + > |
346 | + > Note: this link will expire in about 1 week. If you want to |
347 | + > download these translations again, you will have to request |
348 | + > them again at |
349 | + > |
350 | + > http://translations.launchpad.dev/evolution/trunk/+export |
351 | + > |
352 | + > -- |
353 | + > Automatic message from Launchpad.net... |
354 | |
355 | The email contains a URL linking to where the exported file can be downloaded. |
356 | |
357 | |
358 | === modified file 'lib/lp/translations/doc/poexport-request.txt' |
359 | --- lib/lp/translations/doc/poexport-request.txt 2009-07-23 17:49:31 +0000 |
360 | +++ lib/lp/translations/doc/poexport-request.txt 2009-08-14 17:46:32 +0000 |
361 | @@ -68,25 +68,31 @@ |
362 | |
363 | The user receives a confirmation email. |
364 | |
365 | - >>> def print_mail_body(body): |
366 | - ... lines = body[body.index('\n\n')+2:].split('\n') |
367 | - ... for line in lines: |
368 | - ... print ">", line |
369 | - |
370 | + >>> from lp.translations.tests.helpers import print_mail_subject_and_body |
371 | >>> from lp.services.mail.stub import test_emails |
372 | >>> len(test_emails) |
373 | 1 |
374 | >>> from_addr, to_addrs, body = test_emails.pop() |
375 | >>> to_addrs |
376 | ['mark@hbd.com'] |
377 | - >>> print_mail_body(body) |
378 | - > |
379 | + >>> print_mail_subject_and_body(body) |
380 | + Subject: Launchpad translation download: Ubuntu Hoary pmount - |
381 | + pmount template |
382 | > Hello Mark Shuttleworth, |
383 | > |
384 | > The translation files you requested from Launchpad are ready for |
385 | > download from the following location: |
386 | > |
387 | > http://localhost:58000/.../launchpad-export.tar.gz |
388 | + > |
389 | + > Note: this link will expire in about 1 week. If you want to |
390 | + > download these translations again, you will have to request |
391 | + > them again at |
392 | + > |
393 | + > http://translations.launchpad.../hoary/+source/pmount/+pots/p... |
394 | + > |
395 | + > -- |
396 | + > Automatic message from Launchpad.net... |
397 | |
398 | The email contains a URL linking to where the exported file can be downloaded. |
399 | |
400 | @@ -197,14 +203,26 @@ |
401 | >>> from_addr, to_addrs, body = test_emails.pop() |
402 | >>> to_addrs |
403 | ['mark@hbd.com'] |
404 | - >>> print_mail_body(body) |
405 | - > |
406 | + >>> print_mail_subject_and_body(body) |
407 | + Subject: Launchpad translation download: Ubuntu Hoary pmount - Czech |
408 | + translation of pmount |
409 | > Hello Mark Shuttleworth, |
410 | - > |
411 | + > |
412 | > The translation files you requested from Launchpad are ready for |
413 | > download from the following location: |
414 | - > |
415 | - > http://localhost:58000/.../cs_LC_MESSAGES_pmount.mo |
416 | + > |
417 | + > http://localhost:58000/.../cs_LC_MESSAGES_pmount.mo |
418 | + > |
419 | + > Note: this link will expire in about 1 week. If you want to |
420 | + > download these translations again, you will have to request |
421 | + > them again at |
422 | + > |
423 | + > http://translations.launchpad.../pmount/+pots/pmoun= |
424 | + > t/cs/+export |
425 | + > |
426 | + > -- |
427 | + > Automatic message from Launchpad.net... |
428 | + |
429 | >>> url = extract_url(body) |
430 | |
431 | Check whether we generated a good .mo file. |
432 | |
433 | === added file 'lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt' |
434 | --- lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt 1970-01-01 00:00:00 +0000 |
435 | +++ lib/lp/translations/emailtemplates/poexport-failure-admin-notification.txt 2009-08-14 16:33:57 +0000 |
436 | @@ -0,0 +1,16 @@ |
437 | +Hello admins, |
438 | + |
439 | +Launchpad encountered problems exporting translation files |
440 | +requested by %(person)s (%(person_id)s) at |
441 | + |
442 | + %(request_url)s |
443 | + |
444 | +This means we have a bug in Launchpad that needs to be fixed |
445 | +before this export can proceed. Here is the error we got: |
446 | + |
447 | +%(failure_message)s |
448 | + |
449 | +%(failed_requests)s |
450 | + |
451 | +-- |
452 | +Automatic message from Launchpad.net — https://launchpad.net/ |
453 | |
454 | === added file 'lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt' |
455 | --- lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt 1970-01-01 00:00:00 +0000 |
456 | +++ lib/lp/translations/emailtemplates/poexport-failure-unicodedecodeerror.txt 2009-08-14 16:33:57 +0000 |
457 | @@ -0,0 +1,12 @@ |
458 | +Hello admins, |
459 | + |
460 | +A UnicodeDecodeError occurred while trying to notify you of a |
461 | +failure during a translation export requested by %(person)s |
462 | +(%(person_id)s) at |
463 | + |
464 | + %(request_url)s |
465 | + |
466 | +%(failed_requests)s |
467 | + |
468 | +-- |
469 | +Automatic message from Launchpad.net — https://launchpad.net/ |
470 | |
471 | === added file 'lib/lp/translations/emailtemplates/poexport-failure.txt' |
472 | --- lib/lp/translations/emailtemplates/poexport-failure.txt 1970-01-01 00:00:00 +0000 |
473 | +++ lib/lp/translations/emailtemplates/poexport-failure.txt 2009-08-14 16:33:57 +0000 |
474 | @@ -0,0 +1,12 @@ |
475 | +Hello %(person)s, |
476 | + |
477 | +Launchpad encountered problems exporting the files you requested. |
478 | +The Launchpad Translations team has been notified of this problem. |
479 | +Please reply to this email for further assistance. |
480 | + |
481 | +If you want to retry your request, you can do so at |
482 | + |
483 | + %(request_url)s. |
484 | + |
485 | +-- |
486 | +Automatic message from Launchpad.net — https://launchpad.net/ |
487 | |
488 | === added file 'lib/lp/translations/emailtemplates/poexport-success.txt' |
489 | --- lib/lp/translations/emailtemplates/poexport-success.txt 1970-01-01 00:00:00 +0000 |
490 | +++ lib/lp/translations/emailtemplates/poexport-success.txt 2009-08-14 16:33:57 +0000 |
491 | @@ -0,0 +1,15 @@ |
492 | +Hello %(person)s, |
493 | + |
494 | +The translation files you requested from Launchpad are ready for |
495 | +download from the following location: |
496 | + |
497 | + %(download_url)s |
498 | + |
499 | +Note: this link will expire in about 1 week. If you want to |
500 | +download these translations again, you will have to request |
501 | +them again at |
502 | + |
503 | + %(request_url)s |
504 | + |
505 | +-- |
506 | +Automatic message from Launchpad.net — https://launchpad.net/ |
507 | |
508 | === modified file 'lib/lp/translations/interfaces/translationimportqueue.py' |
509 | --- lib/lp/translations/interfaces/translationimportqueue.py 2009-07-17 00:26:05 +0000 |
510 | +++ lib/lp/translations/interfaces/translationimportqueue.py 2009-08-14 16:35:06 +0000 |
511 | @@ -457,7 +457,7 @@ |
512 | file_type = Choice( |
513 | title=_("File Type"), |
514 | description=_( |
515 | - "The type of the file being imported imported."), |
516 | + "The type of the file being imported."), |
517 | required=True, |
518 | vocabulary = TranslationFileType) |
519 | |
520 | |
521 | === modified file 'lib/lp/translations/scripts/po_export_queue.py' |
522 | --- lib/lp/translations/scripts/po_export_queue.py 2009-07-17 00:26:05 +0000 |
523 | +++ lib/lp/translations/scripts/po_export_queue.py 2009-08-14 18:02:09 +0000 |
524 | @@ -10,7 +10,6 @@ |
525 | |
526 | import os |
527 | import psycopg2 |
528 | -import textwrap |
529 | import traceback |
530 | from StringIO import StringIO |
531 | from zope.component import getAdapter, getUtility |
532 | @@ -18,9 +17,13 @@ |
533 | from canonical.config import config |
534 | from canonical.launchpad import helpers |
535 | from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet |
536 | +from canonical.launchpad.webapp import canonical_url |
537 | +from lp.registry.interfaces.productseries import IProductSeries |
538 | +from lp.registry.interfaces.sourcepackage import ISourcePackage |
539 | from lp.translations.interfaces.poexportrequest import ( |
540 | IPOExportRequestSet) |
541 | from lp.translations.interfaces.potemplate import IPOTemplate |
542 | +from lp.translations.interfaces.pofile import IPOFile |
543 | from lp.translations.interfaces.translationcommonformat import ( |
544 | ITranslationFileData) |
545 | from lp.translations.interfaces.translationexporter import ( |
546 | @@ -35,39 +38,210 @@ |
547 | |
548 | This class has three main attributes: |
549 | |
550 | - - name: A short identifying string for this export. |
551 | + - person: A person requesting this export. |
552 | - url: The Librarian URL for any successfully exported files. |
553 | - - failure: Failure got while exporting. |
554 | + - failure: Failure gotten while exporting. |
555 | """ |
556 | |
557 | - def __init__(self, name): |
558 | - self.name = name |
559 | + def __init__(self, person, requested_exports, logger): |
560 | + self.person = person |
561 | self.url = None |
562 | self.failure = None |
563 | - self.object_names = [] |
564 | - |
565 | - def _getFailureEmailBody(self, person): |
566 | + self.logger = logger |
567 | + |
568 | + self.requested_exports = list(requested_exports) |
569 | + export_requested_at = self._getExportRequestOrigin() |
570 | + self.name = self._getShortRequestName(export_requested_at) |
571 | + |
572 | + self.request_url = canonical_url( |
573 | + export_requested_at, |
574 | + rootsite='translations') + '/+export' |
575 | + |
576 | + def _getShortRequestName(self, request): |
577 | + """Return a short request name for use in email subjects.""" |
578 | + if IPOFile.providedBy(request): |
579 | + title = '%s translation of %s' % ( |
580 | + request.language.englishname, |
581 | + request.potemplate.name) |
582 | + productseries = request.potemplate.productseries |
583 | + distroseries = request.potemplate.distroseries |
584 | + sourcepackagename = request.potemplate.sourcepackagename |
585 | + elif IPOTemplate.providedBy(request): |
586 | + title = '%s template' % (request.name) |
587 | + productseries = request.productseries |
588 | + distroseries = request.distroseries |
589 | + sourcepackagename = request.sourcepackagename |
590 | + elif IProductSeries.providedBy(request): |
591 | + title = None |
592 | + productseries = request |
593 | + distroseries = None |
594 | + sourcepackagename = None |
595 | + elif ISourcePackage.providedBy(request): |
596 | + title = None |
597 | + productseries = None |
598 | + distroseries = request.distroseries |
599 | + sourcepackagename = request.sourcepackagename |
600 | + else: |
601 | + raise AssertionError( |
602 | + "We can not figure out short name for this translation " |
603 | + "export origin.") |
604 | + |
605 | + if productseries is not None: |
606 | + root = '%s %s' % ( |
607 | + productseries.product.displayname, |
608 | + productseries.name) |
609 | + else: |
610 | + root = '%s %s %s' % ( |
611 | + distroseries.distribution.displayname, |
612 | + distroseries.displayname, |
613 | + sourcepackagename.name) |
614 | + if title is not None: |
615 | + return '%s - %s' % (root, title) |
616 | + else: |
617 | + return root |
618 | + |
619 | + def _getExportRequestOrigin(self): |
620 | + """Figure out where an export request was made.""" |
621 | + # Determine all objects that export request could have |
622 | + # originated on. |
623 | + export_requested_at = None |
624 | + pofiles = set() |
625 | + implicit_potemplates = set() |
626 | + direct_potemplates = set() |
627 | + productseries = set() |
628 | + sourcepackages = set() |
629 | + |
630 | + last_template_name = None |
631 | + for request in self.requested_exports: |
632 | + if IPOTemplate.providedBy(request): |
633 | + # If we are exporting a template, add it to |
634 | + # the list of directly requested potemplates. |
635 | + potemplate = request |
636 | + direct_potemplates.add(potemplate) |
637 | + else: |
638 | + # Otherwise, we are exporting a POFile. |
639 | + potemplate = request.potemplate |
640 | + implicit_potemplates.add(potemplate) |
641 | + pofiles.add(request) |
642 | + if potemplate.displayname != last_template_name: |
643 | + self.logger.debug( |
644 | + 'Exporting objects for %s, related to template %s' |
645 | + % (self.person.displayname, potemplate.displayname)) |
646 | + last_template_name = potemplate.displayname |
647 | + |
648 | + # Determine productseries or sourcepackage for any |
649 | + # productseries/sourcepackage an export was requested at. |
650 | + if potemplate.productseries is not None: |
651 | + productseries.add(potemplate.productseries) |
652 | + elif potemplate.sourcepackagename is not None: |
653 | + sourcepackage = potemplate.distroseries.getSourcePackage( |
654 | + potemplate.sourcepackagename) |
655 | + sourcepackages.add(sourcepackage) |
656 | + else: |
657 | + pass |
658 | + |
659 | + if len(pofiles) == 1 and len(direct_potemplates) == 0: |
660 | + # One POFile was requested. |
661 | + export_requested_at = pofiles.pop() |
662 | + elif len(pofiles) == 0 and len(direct_potemplates) == 1: |
663 | + # A POTemplate was requested. |
664 | + export_requested_at = direct_potemplates.pop() |
665 | + elif len(pofiles) + len(direct_potemplates) >= 2: |
666 | + # More than one file was requested. |
667 | + all_potemplates = implicit_potemplates.union(direct_potemplates) |
668 | + if len(all_potemplates) == 1: |
669 | + # It's all part of a single POTemplate. |
670 | + export_requested_at = all_potemplates.pop() |
671 | + else: |
672 | + # More than one POTemplate: request was made on |
673 | + # either ProductSeries or SourcePackage. |
674 | + if len(sourcepackages) > 0: |
675 | + export_requested_at = sourcepackages.pop() |
676 | + elif len(productseries) > 0: |
677 | + export_requested_at = productseries.pop() |
678 | + |
679 | + if IPOTemplate.providedBy(export_requested_at): |
680 | + if len(sourcepackages) > 0: |
681 | + sp = sourcepackages.pop() |
682 | + if sp.getCurrentTranslationTemplates().count() == 1: |
683 | + export_requested_at = sp |
684 | + elif len(productseries) > 0: |
685 | + ps = productseries.pop() |
686 | + if ps.getCurrentTranslationTemplates().count() == 1: |
687 | + export_requested_at = ps |
688 | + |
689 | + return export_requested_at |
690 | + |
691 | + |
692 | + def _getRequestedExportsNames(self): |
693 | + requested_names = [] |
694 | + for translation_object in self.requested_exports: |
695 | + if IPOTemplate.providedBy(translation_object): |
696 | + request_name = translation_object.displayname |
697 | + else: |
698 | + request_name = translation_object.title |
699 | + requested_names.append(request_name) |
700 | + |
701 | + return requested_names |
702 | + |
703 | + def _getFailureEmailBody(self): |
704 | """Send an email notification about the export failing.""" |
705 | - return textwrap.dedent(''' |
706 | - Hello %s, |
707 | - |
708 | - Launchpad encountered problems exporting the files you requested. |
709 | - The Launchpad Translations team has been notified of this problem. |
710 | - Please reply to this email for further assistance. |
711 | - ''' % person.displayname) |
712 | - |
713 | - def _getSuccessEmailBody(self, person): |
714 | + template = helpers.get_email_template( |
715 | + 'poexport-failure.txt', 'translations').decode('utf-8') |
716 | + return template % { |
717 | + 'person' : self.person.displayname, |
718 | + 'request_url' : self.request_url, |
719 | + } |
720 | + |
721 | + def _getFailedRequestsDescription(self): |
722 | + """Return a printable description of failed export requests.""" |
723 | + failed_requests = self._getRequestedExportsNames() |
724 | + if len(failed_requests) > 0: |
725 | + failed_requests_text = 'Failed export request included:\n' |
726 | + failed_requests_text += '\n'.join( |
727 | + [' * ' + request for request in failed_requests]) |
728 | + else: |
729 | + failed_requests_text = 'There were no export requests.' |
730 | + return failed_requests_text |
731 | + |
732 | + def _getAdminFailureNotificationEmailBody(self): |
733 | + """Send an email notification about failed export to admins.""" |
734 | + template = helpers.get_email_template( |
735 | + 'poexport-failure-admin-notification.txt', |
736 | + 'translations').decode('utf-8') |
737 | + failed_requests = self._getFailedRequestsDescription() |
738 | + return template % { |
739 | + 'person' : self.person.displayname, |
740 | + 'person_id' : self.person.name, |
741 | + 'request_url' : self.request_url, |
742 | + 'failure_message' : self.failure, |
743 | + 'failed_requests' : failed_requests, |
744 | + } |
745 | + |
746 | + def _getUnicodeDecodeErrorEmailBody(self): |
747 | + """Send an email notification to admins about UnicodeDecodeError.""" |
748 | + template = helpers.get_email_template( |
749 | + 'poexport-failure-unicodedecodeerror.txt', |
750 | + 'translations').decode('utf-8') |
751 | + failed_requests = self._getFailedRequestsDescription() |
752 | + return template % { |
753 | + 'person' : self.person.displayname, |
754 | + 'person_id' : self.person.name, |
755 | + 'request_url' : self.request_url, |
756 | + 'failed_requests' : failed_requests, |
757 | + } |
758 | + |
759 | + def _getSuccessEmailBody(self): |
760 | """Send an email notification about the export working.""" |
761 | - return textwrap.dedent(''' |
762 | - Hello %s, |
763 | - |
764 | - The translation files you requested from Launchpad are ready for |
765 | - download from the following location: |
766 | - |
767 | - \t%s''' % (person.displayname, self.url) |
768 | - ) |
769 | - |
770 | - def notify(self, person): |
771 | + template = helpers.get_email_template( |
772 | + 'poexport-success.txt', 'translations').decode('utf-8') |
773 | + return template % { |
774 | + 'person' : self.person.displayname, |
775 | + 'download_url' : self.url, |
776 | + 'request_url' : self.request_url, |
777 | + } |
778 | + |
779 | + def notify(self): |
780 | """Send a notification email to the given person about the export. |
781 | |
782 | If there is a failure, a copy of the email is also sent to the |
783 | @@ -76,22 +250,22 @@ |
784 | if self.failure is None and self.url is not None: |
785 | # There is no failure, so we have a full export without |
786 | # problems. |
787 | - body = self._getSuccessEmailBody(person) |
788 | + body = self._getSuccessEmailBody() |
789 | elif self.failure is not None and self.url is None: |
790 | - body = self._getFailureEmailBody(person) |
791 | + body = self._getFailureEmailBody() |
792 | elif self.failure is not None and self.url is not None: |
793 | raise AssertionError( |
794 | 'We cannot have a URL for the export and a failure.') |
795 | else: |
796 | raise AssertionError('On success, an exported URL is expected.') |
797 | |
798 | - recipients = list(helpers.get_contact_email_addresses(person)) |
799 | + recipients = list(helpers.get_contact_email_addresses(self.person)) |
800 | |
801 | for recipient in [str(recipient) for recipient in recipients]: |
802 | simple_sendmail( |
803 | from_addr=config.rosetta.admin_email, |
804 | to_addrs=[recipient], |
805 | - subject='Translation download request: %s' % self.name, |
806 | + subject='Launchpad translation download: %s' % self.name, |
807 | body=body) |
808 | |
809 | if self.failure is None: |
810 | @@ -99,42 +273,20 @@ |
811 | return |
812 | |
813 | # The export process had errors that we should notify admins about. |
814 | - if self.object_names: |
815 | - names = '\n'.join(self.object_names) |
816 | - template_sentence = "\n" + textwrap.dedent( |
817 | - "The failed request involved these objects:\n%s" % names) |
818 | - else: |
819 | - template_sentence = "" |
820 | - |
821 | try: |
822 | - admins_email_body = textwrap.dedent(''' |
823 | - Hello admins, |
824 | - |
825 | - Launchpad encountered problems exporting translation files |
826 | - requested by %s. |
827 | - |
828 | - This means we have a bug in Launchpad that needs to be fixed |
829 | - before this export can proceed. Here is the list of failed |
830 | - files and the error we got: |
831 | - |
832 | - %s%s''') % ( |
833 | - person.displayname, self.failure, template_sentence) |
834 | + admins_email_body = self._getAdminFailureNotificationEmailBody() |
835 | except UnicodeDecodeError: |
836 | # Unfortunately this happens sometimes: invalidly-encoded data |
837 | # makes it into the exception description, possibly from error |
838 | # messages printed by msgfmt. Before we can fix that, we need to |
839 | # know what exports suffer from this problem. |
840 | - admins_email_body = textwrap.dedent(''' |
841 | - Hello admins, |
842 | - |
843 | - A UnicodeDecodeError occurred while trying to notify you of a |
844 | - failure during a translation export requested by %s. |
845 | - %s''') % (person.displayname, template_sentence) |
846 | + admins_email_body = self._getUnicodeDecodeErrorEmailBody() |
847 | |
848 | simple_sendmail( |
849 | from_addr=config.rosetta.admin_email, |
850 | to_addrs=[config.launchpad.errors_address], |
851 | - subject='Translation download errors: %s' % self.name, |
852 | + subject=( |
853 | + 'Launchpad translation download errors: %s' % self.name), |
854 | body=admins_email_body) |
855 | |
856 | def addFailure(self): |
857 | @@ -174,26 +326,11 @@ |
858 | translation_format_exporter = ( |
859 | translation_exporter.getExporterProducingTargetFileFormat(format)) |
860 | |
861 | - result = ExportResult(person.name) |
862 | - translation_file_list = list(objects) |
863 | - last_template_name = None |
864 | - for obj in translation_file_list: |
865 | - if IPOTemplate.providedBy(obj): |
866 | - template_name = obj.displayname |
867 | - object_name = template_name |
868 | - else: |
869 | - template_name = obj.potemplate.displayname |
870 | - object_name = obj.title |
871 | - result.object_names.append(object_name) |
872 | - if template_name != last_template_name: |
873 | - logger.debug( |
874 | - 'Exporting objects for %s, related to template %s' |
875 | - % (person.displayname, template_name)) |
876 | - last_template_name = template_name |
877 | + result = ExportResult(person, objects, logger) |
878 | |
879 | try: |
880 | exported_file = translation_format_exporter.exportTranslationFiles( |
881 | - generate_translationfiledata(translation_file_list, format)) |
882 | + generate_translationfiledata(list(objects), format)) |
883 | except (KeyboardInterrupt, SystemExit): |
884 | # We should never catch KeyboardInterrupt or SystemExit. |
885 | raise |
886 | @@ -227,7 +364,7 @@ |
887 | result.url = alias.http_url |
888 | logger.info("Stored file at %s" % result.url) |
889 | |
890 | - result.notify(person) |
891 | + result.notify() |
892 | |
893 | |
894 | def process_queue(transaction_manager, logger): |
895 | |
896 | === added file 'lib/lp/translations/tests/helpers.py' |
897 | --- lib/lp/translations/tests/helpers.py 1970-01-01 00:00:00 +0000 |
898 | +++ lib/lp/translations/tests/helpers.py 2009-08-14 17:46:32 +0000 |
899 | @@ -0,0 +1,19 @@ |
900 | +# Copyright 2009 Canonical Ltd. This software is licensed under the |
901 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
902 | + |
903 | +"""Helper module reused in different tests.""" |
904 | + |
905 | +__metaclass__ = type |
906 | + |
907 | +__all__ = [ |
908 | + 'print_mail_subject_and_body' |
909 | + ] |
910 | + |
911 | +import email |
912 | + |
913 | +def print_mail_subject_and_body(contents): |
914 | + msg = email.message_from_string(contents) |
915 | + body = msg.get_payload() |
916 | + print 'Subject: %s' % (msg['subject']) |
917 | + for line in body.split('\n'): |
918 | + print ">", line |
919 |
= Summary =
This branch fixes #92751 (#410579 is a duplicate which I originally
started on), doing a bunch of drive-by cleanups along the way.
Our current download email for translations is very terse and not very
informative. It goes something like the following:
Subject: Translation download request: danilo launchpadlibrar ian.net/ 30048422/ po_kio4- es.po
> Hello Данило Шеган,
>
> The translation files you requested from Launchpad are ready for
> download from the following location:
>
> http://
== Proposed fix ==
We should include more information, like what the request is for, and
how download link will expire in 1 week. We should also include
information on how you can re-request the download if you need to.
Subject should not mention your username, but instead a short
description of what the download is about.
== Implementation details ==
The branch has quickly gotten very big (~900 lines), but it's mostly
because of textual changes to email templates. I hope it's not a problem.
* scripts/ po_export_ queue.py:
This is where the core change is: everything related to emailing is moved into
ExportResult implementation, which generates appropriate emails.
Two methods are of notice:
* _getExportReque stOrigin( ): figures out where export request was most
TranslationImpo rtQueue keeps only a list of POTemplate and POFile objects
likely made;
and depending this method figures out the place where the request was
"most likely" made (i.e. context where you could have requested these
files, but then trying for the most relevant context as well).
For instance, if we've got two pofiles on a single template, request was sourcepackage, we return them instead).
likely made on a potemplate object, so we return that (for nicer URLs
and emails subjects though, if there's only one template in
a productseries/
* _getShortReques tName(request) takes the context returned by previous method
and returns a nice short description in the form of 'Evolution trunk' or
'Ubuntu Karmic gimp-2.0 - Spanish translation' suitable for use as email
subjects
These are not unit tested, though they could be, simply because they are
sufficiently well exercised by existing doctests, and because if these start
failing, it'll be easy to notice (so, I'd rather not increase CPU cycles
we spend on tests). They are mostly for 'nicer UI output', so their correctness
is not critical to the operation.
Other changes in this file are mostly for using emailtemplates instead of hardcoding text.
* emailtemplates/ poexport- *.txt:
All the email templates have been moved out of the code and into separate
template files. Some 75 lines of diff are basically
* doc/poexport- request. txt, poexport- request- productseries. txt, poexport- queue.txt: subject_ and_body method,
doc/
doc/
There are a lot of changes to tests that test how emails are generated:
basically, half of the branch is there (440 lines). The changes there
include introducing a shared helper print_mail_
print subject along with body of the email, and changes to test new
e...