Sure, go ahead and start a separate thread. Can I ask that for issues you find while hacking the community copy can you open jiras for me, as I do plan to incorporate all the changes you are making, just that I want them to go through the review process first.
-Justin Daniele Romagnoli wrote: > Being the RESTConfig community module no more used, we can remove it and > copy the modified restconfig in community. (do you think about a special > name to avoid confusion? some prefix/suffix?) > I think the new restconfig has several improvements with respect to the > older one, therefore we prefer to use the last one. > If we go in that direction, you can clean up the extension as you like. > Let me know and I will open a new thread to request setting up a > community module. > > Cheers, > Daniele > > > > On Fri, May 8, 2009 at 6:40 PM, Justin Deoliveira <[email protected] > <mailto:[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]> > <mailto:[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]> > <mailto:[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]> > <mailto:[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]>> > >>>> <mailto:[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]>> > >>>> <mailto:[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]> > <mailto:[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]> > <mailto:[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. > > > > > -- > ------------------------------------------------------- > 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
