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]> wrote:
> Ciao Justin,
> please, see below...
>
>
> On Fri, May 8, 2009 at 6:10 PM, Justin Deoliveira <[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]
> >
> >> 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]>> 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
> >>>
> >
> >
> > --
> > 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
