On Fri, May 8, 2009 at 5:25 PM, Justin Deoliveira <[email protected]>wrote:
> For the first patch:
>
> + final LayerInfo layerInfo;
> + final Map<String,String> layerProperties = new
> HashMap<String,String>(2);
> + if (form.getFirst("style") != null)
> + layerProperties.put("style",
> form.getFirstValue("style"));
> + if (form.getFirst("wmspath") != null)
> + layerProperties.put("path",
> form.getFirstValue("wmspath"));
> + if (layerProperties.isEmpty())
> + layerInfo = builder.buildLayer(cinfo);
> + else
> + layerInfo = builder.buildLayer(cinfo,layerProperties);
> + catalog.add(layerInfo);
>
> I think this should be handled with a second call, rather than stack up
> available options. It makes the api cleaner imo. And is less arbitrary. For
> instance why do we support style and wms path but not the other properties?
>
> So I would prefer the interaction to be:
>
> 1) PUT to upload the coverage
> 2) PUT to modify the coverage
>
> Objections?
In my opinion, I think it would be better to do these quick additional
settings in a single pass.
About your question involving properties, indeed, we can check for
additional properties.
Daniele
>
>
> Daniele Romagnoli wrote:
>
>> Hi Justin,
>> I'm really sorry about this. Next time I'll follows the steps you are
>> talking about.
>> I was working on the coverages ingestion and I needed to re-enable some
>> capabilities available in 1.7.2 for an urgent application.
>> I was too hasty. Sorry again.
>>
>>
>> On Fri, May 8, 2009 at 4:22 PM, Justin Deoliveira
>> <[email protected]<mailto:
>> [email protected]>> wrote:
>>
>> Reviewing the commits more closely I definitely have a few concerns
>> which affirms my preference to have these changes reviewed before hand.
>>
>> * The method added to catalog builder seems unnecessary to me. I think
>> just the plain buildLayer can be called, and then properties overrideen
>> after the fact, instead put putting them into a map and passing it into
>> the method. It also does not sync up with any of the other methods in
>> that class. I would like to keep that class as consistent as possible.
>>
>> * You have made changes to api without any discussion. Supporting
>> wmspath and and style as query parameters is not documented as api
>> options:
>>
>>
>> http://gridlock.openplans.org/geoserver/1.7.x/doc/user/extensions/rest/rest-config-api.html#coverage-stores
>>
>>
>> The previous RESTConfig allowed to handle them. I had tought during
>> migration to extensions they have been forgotten.
>>
>> <
>> http://gridlock.openplans.org/geoserver/1.7.x/doc/user/extensions/rest/rest-config-api.html#coverage-stores
>> >
>>
>> Not saying we should not have them, but this is an extension now, not a
>> community module anymore, such things require discussion before hand.
>>
>>
>> You are Right.
>>
>>
>>
>> * Many of the changes seem a bit arbitrary, changing variable
>> declarations to final for instance. Again, for consistency sake I
>> prefer
>> that you delegate to how the rest of the class / method does things
>> when
>> making chagnes.
>>
>> * Setting the layer name as the entity is again an api change, and one
>> I
>> don't agree with. It is more RESTful to set the location of the
>> resource
>> using the appropriate HTTP header, 'Location' like we do for the POST
>> case. Also, the indication of where the resource should be down with a
>> full uri, just returning the name assumes that URI's are non-opaque, ie
>> the client has pre-knowledge of how URI's are built, which is another
>> REST nono.
>>
>>
>> Do you think it would be better if I revert my changes?
>> Daniele
>>
>>
>>
>>
>> -Justin
>>
>> Justin Deoliveira wrote:
>> > Hi Daniele,
>> >
>> > I see you are making commits on 1.7.x to restconfig and main. I
>> would
>> > appreciate posting patches for pre review first before committing.
>> > Especially in this case where we have not yet wrapped up discussion
>> > about how to handle the PUT issue from previous email.
>> >
>> > I am not trying to hamper work here but I have been deemed the
>> > maintainer of that extension, and while there are no hard rules
>> in place
>> > about who is allowed to commit, i would appreciate being to review
>> > commits before they go through.
>> >
>> > -Justin
>> >
>>
>>
>> --
>> Justin Deoliveira
>> OpenGeo - http://opengeo.org
>> Enterprise support for open source geospatial.
>>
>>
>>
>> ------------------------------------------------------------------------------
>> The NEW KODAK i700 Series Scanners deliver under ANY circumstances!
>> Your
>> production scanning environment may not be a perfect world - but
>> thanks to
>> Kodak, there's a perfect scanner to get the job done! With the NEW
>> KODAK i700
>> Series Scanner you'll get full speed at 300 dpi even with all image
>> processing features enabled. http://p.sf.net/sfu/kodak-com
>> _______________________________________________
>> Geoserver-devel mailing list
>> [email protected]
>> <mailto:[email protected]>
>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>>
>>
>>
>>
>> --
>> -------------------------------------------------------
>> Eng. Daniele Romagnoli
>> Software Engineer
>>
>> GeoSolutions S.A.S.
>> Via Carignoni 51
>> 55041 Camaiore (LU)
>> Italy
>>
>> phone: +39 0584983027
>> fax: +39 0584983027
>> mob: +39 328 0559267
>>
>>
>> http://www.geo-solutions.it
>>
>> -------------------------------------------------------
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>> ------------------------------------------------------------------------------
>> The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your
>> production scanning environment may not be a perfect world - but thanks to
>> Kodak, there's a perfect scanner to get the job done! With the NEW KODAK
>> i700
>> Series Scanner you'll get full speed at 300 dpi even with all image
>> processing features enabled. http://p.sf.net/sfu/kodak-com
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Geoserver-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/geoserver-devel
>>
>
>
> --
> Justin Deoliveira
> OpenGeo - http://opengeo.org
> Enterprise support for open source geospatial.
>
>
--
-------------------------------------------------------
Eng. Daniele Romagnoli
Software Engineer
GeoSolutions S.A.S.
Via Carignoni 51
55041 Camaiore (LU)
Italy
phone: +39 0584983027
fax: +39 0584983027
mob: +39 328 0559267
http://www.geo-solutions.it
-------------------------------------------------------
------------------------------------------------------------------------------
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your
production scanning environment may not be a perfect world - but thanks to
Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700
Series Scanner you'll get full speed at 300 dpi even with all image
processing features enabled. http://p.sf.net/sfu/kodak-com
_______________________________________________
Geoserver-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
