Ciao Justin, just out of curiosity, what's the situation with rest and restconfig on trunk?
Simone. ------------------------------------------------------- Ing. Simone Giannecchini GeoSolutions S.A.S. Owner - Software Engineer Via Carignoni 51 55041 Camaiore (LU) Italy phone: +39 0584983027 fax: +39 0584983027 mob: +39 333 8128928 http://www.geo-solutions.it http://simboss.blogspot.com/ http://www.linkedin.com/in/simonegiannecchini ------------------------------------------------------- On Fri, May 8, 2009 at 6:40 PM, Justin Deoliveira <[email protected]> wrote: > Well actually thinking about it, the old RESTConfig community module is > still there, and all the classes are there pre-change. So you may just be > able to use it. > > My only hesitation is that we will end up in this same situation, a bunch of > work will be done there and it will not be ported properly back to the > extension. But since you guys are on a tight deadline we don't have much > choice. > > So proceed as you see fit, either create a new community module or use the > existing RESTConfig one. > > -Justin > > Daniele Romagnoli wrote: >> >> Hi, >> while waiting for an improved catalog, as a quick workaround we could set >> up a community module which is a "copy" of the actual restconfig extension >> containing the workarounds. >> (let's say, something similar to a branch, but restricted to a single >> module). >> >> Does this sounds ok? >> Daniele >> >> >> >> On Fri, May 8, 2009 at 6:16 PM, Simone Giannecchini >> <[email protected] >> <mailto:[email protected]>> wrote: >> >> Ciao Justin, >> please, see below... >> >> >> On Fri, May 8, 2009 at 6:10 PM, Justin Deoliveira >> <[email protected] <mailto:[email protected]>> wrote: >> > >> > >> > Simone Giannecchini wrote: >> >> >> >> Actually, >> >> yeah, I have one objection. >> >> Beside the fact that I am not so convinced that separating the two >> >> calls will make the api cleaner, >> > >> > I would argue it does from a REST perspective. Any parameters >> that modify >> > the resource being PUT should be in the request entity, not in >> the URI that >> > defines the resource. >> > >> > I am quite worried that this would >> >> >> >> make the whole process much weaker. In the real world, when you >> want >> >> to add data in a dynamic way with a heavily loaded server with the >> >> current geoserver internal catalog it is not uncommon that >> everything >> >> breaks due to the fact that it takes quite some time to persiste >> the >> >> changes. You understand that starting to explode the number of >> calls >> >> to configure a single layer will not work in real use cases. >> > >> > I understand the issue, but I hope you can understand why I am >> hesitant to >> > make API compromises to get around limitations which are >> currently just an >> > implementation detail. Like if we had a catalog that performed >> well under >> > update this would be an non issue. >> > >> >> I agree, but we don't have such a catalog yet :-). >> >> > Even in 2.0 this should be much less of an issue since now the >> only thing >> > that gets saved is what you modify. Should be much more >> performant under >> > heavy update load. That said I do realize that you are working >> with 1.7.x >> > and need to work stable. >> > >> > Putting the issue of the current catalog aside for the moment, I >> would >> > speculate that the cost of a second call should be negligible >> compared to >> > the first. The reason being that since the first request involves >> a data >> > upload it could potentially take a while assuming a decently >> sized dataset. >> > But the second call should always be constant time. So you have 1 >> > potentially long request + 1 constant time (instantaneous when we >> have a >> > decent catalog). >> > >> >> I am talking about a different thing. >> If you have 2000 coveragerages configured it takes a non negligible >> time to persist the config. >> Do it twice is simply not acceptable. >> >> >> >> > That said, I would like to come up with a compromise here, and I >> would like >> > to do so in a way that does not prevent you from meeting any >> deadlines. >> > Since the rest system is pluggable can we move the code that >> contains these >> > workarounds for our current catalog limitations into a community >> module. >> > Doing so allows us to have a way to achieve the desired >> functionality for >> > now, but at the same time does not tie us to it for future >> versions. And >> > with a little spring magic we should be able to do in a way that the >> > community module version overrides the extension version so there >> are no URI >> > conflicts. >> >> I'll live that to daniele, since we already modifying code that worked >> before 1.7.3, therefore there is not much time available for more >> work. >> >> Simone. >> > >> > Thoughts? >> >> >> >> >> >> Simone. >> >> ------------------------------------------------------- >> >> Ing. Simone Giannecchini >> >> GeoSolutions S.A.S. >> >> Owner - Software Engineer >> >> Via Carignoni 51 >> >> 55041 Camaiore (LU) >> >> Italy >> >> >> >> phone: +39 0584983027 >> >> fax: +39 0584983027 >> >> mob: +39 333 8128928 >> >> >> >> >> >> http://www.geo-solutions.it >> >> http://simboss.blogspot.com/ >> >> http://www.linkedin.com/in/simonegiannecchini >> >> >> >> ------------------------------------------------------- >> >> >> >> >> >> >> >> On Fri, May 8, 2009 at 5:25 PM, Justin Deoliveira >> <[email protected] <mailto:[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? >> >>> >> >>> 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]> >> >>>> <mailto:[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]> >> >>>> <mailto:[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] >> <mailto:[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] >> <mailto:[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 >> >> ------------------------------------------------------- >> > > > -- > 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
