Re: [Maposmatic-dev] [PATCH maposmatic 2/2] Implement the "Recreate Map" feature

[Maxime Petazzoni]

Hi,

* David MENTRE <address@hidden> [2010-06-01 17:10:29]:

> 2010/5/14 Maxime Petazzoni <address@hidden>:
> > Each completed job now features a "Recreate map" button that reschedules
> > a similar map on the queue. The button only appears for completed maps
> > (status >= 2), and triggers a rendering_already_exists() check before
> > creating a new MapRenderingJob.
> 
> In fact, if I have read correctly your
> rendering_already_exists_by_bbox() code, the button only appears for
> status==2 maps.
> Instead of 2, maybe a symbolic constant would be better.

You haven't, but I understand why you're mistaken :)

The status==2 in the filter is to find an already existing, successful
rendering we can redirect to. And since the query is dependant on the
table's structure anyway, it's not /that/ important to use a constant
here (and we of course can't call .is_ok() for example).

Note that I didn't add/change this. It's barely a duplicate of the
already existing rendering_already_exists code that looked for
successful renderings by administrative boundary and which already used
status==2 in its query.

The rendering_already_exists_by_{bbox,osmid}() code has no effect on the
appearance of the button. The buttons appears when the job is not in the
"needs waiting" status, i.e. when the job has been processed, had it
failed or succeeded (see templates/maposmatic/job.html in the patch).


Other than that, is the patch ok for merging?

Thanks for the review,
- Maxime
-- 
Maxime Petazzoni <http://www.bulix.org>
 ``One by one, the penguins took away my sanity.''
Linux kernel and software developer at MontaVista Software

signature.asc
Description: Digital signature