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?
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.
------------------------------------------------------------------------------
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
