Re: [Maposmatic-dev] [PATCH 2/6] Implement a skeleton /papersize/ service

[Maxime Petazzoni]

Hi Thomas,

* Thomas Petazzoni <address@hidden> [2010-08-05 00:54:23]:

> This service receives by post either a (osmid, layout) or a (bbox,
> layout) and is responsible for asking OCitySMap what are the possible
> paper size for the given geographic area. As OCitySMap doesn't
> implement this feature yet, it is only a skeleton that returns a fixed
> set of paper sizes.

As the number of "POST services" grow, I think it would be better to put
these in /apis/nominatim/ and /apis/papersize/ (we can easily imagine
that we'll have a /apis/newmap later down the road for example). This
way, all JSON-returning APIs would be clearly identified in this
namespace and not polute the "public URLs".

>  from django.utils.translation import ugettext_lazy as _
> +from ocitysmap.coords import BoundingBox as OCMBoundingBox

Why not simply: import ocitysmap.coords, and later:
coords.BoundingBox(...) ?

> +def query_papersize(request):
> +    if request.method == 'POST':
> +        osmid            = request.POST.get('osmid', None)
> +        layout           = request.POST.get('layout', None)
> +        lat_upper_left   = request.POST.get('lat_upper_left', None)
> +        lon_upper_left   = request.POST.get('lon_upper_left', None)
> +        lat_bottom_right = request.POST.get('lat_bottom_right', None)
> +        lon_bottom_right = request.POST.get('lon_bottom_right', None)

Isn't it possible to use a Django form here, even though the form isn't
actually rendered anywhere?

> +        assert layout is not None

Don't use assert here, it will spit out an AssertionError and we don't
want that. Instead, return an HttpResponseBadRequest (see
query_nominatim or recreate).

Rest looks good to me.

- 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