Dear Devon,
some feedback on the proposal:

-1- I would rename GranuleGeometryHandler to GranuleHandler (or
something GranuleiNDEXHandler) like sine one can pretty much do
anything  with this
-2- I would want two different methods just one like:

void handleGeometry(
            GridCoverage2DReader inputReader,
            SimpleFeature targetFeature,
            SimpleFeatureType targetFeatureType,
            SimpleFeature inputFeature,
            SimpleFeatureType inputFeatureType,
            MosaicConfigurationBean mosaicConfiguration);

and some code in it to differentiate makes things simpler

-3- I am wondering if we might want to have multiple GranuleHandlers
like we have for the other SPIs, probably not...

I will send you a PR with some feedback along these lines; since we
are here, we need some unit tests for the new classes we have created.
With this feedback fixed you have my +1.

Regards,
Simone Giannecchini
==
GeoServer Professional Services from the experts!
Visit http://goo.gl/it488V for more information.
==
Ing. Simone Giannecchini
@simogeo
Founder/Director

GeoSolutions S.A.S.
Via di Montramito 3/A
55054  Massarosa (LU)
Italy
phone: +39 0584 962313
fax:     +39 0584 1660272
mob:   +39  333 8128928

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
Le informazioni contenute in questo messaggio di posta elettronica e/o
nel/i file/s allegato/i sono da considerarsi strettamente riservate.
Il loro utilizzo è consentito esclusivamente al destinatario del
messaggio, per le finalità indicate nel messaggio stesso. Qualora
riceviate questo messaggio senza esserne il destinatario, Vi preghiamo
cortesemente di darcene notizia via e-mail e di procedere alla
distruzione del messaggio stesso, cancellandolo dal Vostro sistema.
Conservare il messaggio stesso, divulgarlo anche in parte,
distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità
diverse, costituisce comportamento contrario ai principi dettati dal
D.Lgs. 196/2003.

The information in this message and/or attachments, is intended solely
for the attention and use of the named addressee(s) and may be
confidential or proprietary in nature or covered by the provisions of
privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
Data Protection Code).Any use not in accord with its purpose, any
disclosure, reproduction, copying, distribution, or either
dissemination, either whole or partial, is strictly forbidden except
previous formal approval of the named addressee(s). If you are not the
intended recipient, please contact immediately the sender by
telephone, fax or e-mail and delete the information in this message
that has been received in error. The sender does not give any warranty
or accept liability as the content, accuracy or completeness of sent
messages and accepts no responsibility  for changes made after they
were sent or for other risks which arise as a result of e-mail
transmission, viruses, etc.


On Wed, Jun 22, 2016 at 6:31 AM, Jody Garnett <jody.garn...@gmail.com> wrote:
> Thanks, proposal is clear, scope/purpose clear, technical details are easier
> to follow with class diagram.
>
> +1
>
> And it looks like you are pretty much done the work, except for user docs...
>
> --
> Jody Garnett
>
> On 21 June 2016 at 17:04, Devon Tucker <devonrtuc...@gmail.com> wrote:
>>
>> Yes, it's ready for review :D
>>
>> On Tue, Jun 21, 2016 at 5:01 PM, Jody Garnett <jody.garn...@gmail.com>
>> wrote:
>>>
>>> We can always squash the commits when merging to master.
>>>
>>> Is the proposal ready for review now?
>>>
>>> --
>>> Jody Garnett
>>>
>>> On 21 June 2016 at 16:55, Devon Tucker <devonrtuc...@gmail.com> wrote:
>>>>
>>>> Ok so after going down the road of using the PropertiesCollector
>>>> interface for doing the granule geometry handling and decided to go with my
>>>> original plan of creating a separate interface for this. I have updated the
>>>> proposal accordingly.
>>>>
>>>> Also, I created a pull request for reference, since it might be easier
>>>> to follow these changes that way.
>>>>
>>>> https://github.com/geotools/geotools/pull/1220
>>>>
>>>> I can spend some time cleaning up those commits, since I notice now that
>>>> there's a few checkpoint commits in there.
>>>>
>>>> Cheers
>>>>
>>>> On Mon, Jun 20, 2016 at 5:06 PM, Devon Tucker <devonrtuc...@gmail.com>
>>>> wrote:
>>>>>
>>>>> The GranuleAcceptor stuff is done on that branch. I have the
>>>>> PropertiesCollector stuff done locally, but there's some iffy stuff there.
>>>>> Handling the granule envelope in a PropertiesCollector should be fine, but
>>>>> there is an issue with how StructuredGridCoverages are handled:
>>>>>
>>>>>
>>>>> https://github.com/dvntucker/geotools/blob/granule_acceptors/modules/plugin/imagemosaic/src/main/java/org/geotools/gce/imagemosaic/ImageMosaicConfigHandler.java#L512
>>>>>
>>>>> For StructuredGCs all the feature attributes are copied right off the
>>>>> feature from the input reader, then the properties collectors are called.
>>>>> I'm not really sure how this would tie in to using a PropertiesCollector 
>>>>> the
>>>>> pre-process the incoming coverage envelope, which makes me wonder if it's 
>>>>> a
>>>>> good idea at all.
>>>>>
>>>>>
>>>>>
>>>>> On Mon, Jun 20, 2016 at 2:53 PM, Jody Garnett <jody.garn...@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> Reviewing
>>>>>> https://github.com/geotools/geotools/wiki/Refactor-ImageMosaic-Index-and-Catalog-management-for-improved-extensibility
>>>>>> it appears up to date (with a remove CatalogManager heading). Are you 
>>>>>> going
>>>>>> to verify the approach (on that branch) and then send us an email for the
>>>>>> completed proposal?
>>>>>>
>>>>>> It is hard to leap into this proposal and provide any useful feedback
>>>>>> as a PMC member, I think the productive conversation is with Simone as
>>>>>> module maintainer.
>>>>>> --
>>>>>> Jody
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Jody Garnett
>>>>>>
>>>>>> On 20 June 2016 at 13:48, Devon Tucker <devonrtuc...@gmail.com> wrote:
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Based on discussions with Simone and Jody we have made a few changes
>>>>>>> to this proposal:
>>>>>>>
>>>>>>> - CatalogManager gets deleted, its methods moved either to
>>>>>>> ImageMosaicConfigHandler, Utils, or the GranuleCatalog implementations
>>>>>>>
>>>>>>> - Instead of delegating granule acceptance to CatalogManager, as was
>>>>>>> originally proposed, this functionality is broken out into its own 
>>>>>>> interface
>>>>>>> + factory SPI.
>>>>>>>
>>>>>>> - I changed the granule envelope handling to just use the existing
>>>>>>> PropertiesCollector API, since it already sets attributes on the index
>>>>>>> feature, I don't really see why not? Not sure if anyone would object to
>>>>>>> this.
>>>>>>>
>>>>>>> Also there's a new branch with most of this implemented:
>>>>>>>
>>>>>>> https://github.com/dvntucker/geotools/tree/granule_acceptors
>>>>>>>
>>>>>>> As usual, let me know if there's any questions or feedback.
>>>>>>>
>>>>>>> On Wed, Jun 15, 2016 at 5:38 AM, Simone Giannecchini
>>>>>>> <simone.giannecch...@geo-solutions.it> wrote:
>>>>>>>>
>>>>>>>> Dear Devon,
>>>>>>>> a few things:
>>>>>>>>
>>>>>>>> -0- we should really get rid of this CatalogManager as is (a class
>>>>>>>> with only static methods as its state its spread over N other
>>>>>>>> classes)
>>>>>>>> -1- we can already mosaick images with different colormodels (to a
>>>>>>>> certain extent, it does not make sense to mosaick a float raster
>>>>>>>> with
>>>>>>>> an RGB). RGB, Gray and Palette can be mosaicked as of today
>>>>>>>> -4- I would add some UML diagrams to your proposal to show what we
>>>>>>>> have now and what we have afterwards
>>>>>>>>
>>>>>>>> I noticed that you are half-way through with your refactor and its
>>>>>>>> probably too early to look into it but I have two things I wanted to
>>>>>>>> bring up.
>>>>>>>> First of all, I believe you shuold look at the refactor of the
>>>>>>>> indexing together with this, I would not split them otherwise your
>>>>>>>> refactor in this case could be too narrow.
>>>>>>>> Let me explain, your goal here is to improve the harvesting process
>>>>>>>> allowing implementors to change how harvested elements are discarded
>>>>>>>> as well as how they are preprocessed or manipulated and this is not
>>>>>>>> isolated from the harvesting itself.
>>>>>>>>
>>>>>>>> I believe catalogmanager (although) the nice name, might go away and
>>>>>>>> this kind of behavior could be isolated into smaller API likewise we
>>>>>>>> did with the properties collector so that one can drop them inside
>>>>>>>> the
>>>>>>>> imagemosaic and ask it through the indexmanager to apply them.
>>>>>>>> Ideally one could rather write its one harvester and completely
>>>>>>>> customize how we put things inside the GranuleCatalog.
>>>>>>>>
>>>>>>>> Let me know what you think about this.
>>>>>>>>
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Simone Giannecchini
>>>>>>>> ==
>>>>>>>> GeoServer Professional Services from the experts!
>>>>>>>> Visit http://goo.gl/it488V for more information.
>>>>>>>> ==
>>>>>>>> Ing. Simone Giannecchini
>>>>>>>> @simogeo
>>>>>>>> Founder/Director
>>>>>>>>
>>>>>>>> GeoSolutions S.A.S.
>>>>>>>> Via di Montramito 3/A
>>>>>>>> 55054  Massarosa (LU)
>>>>>>>> Italy
>>>>>>>> phone: +39 0584 962313
>>>>>>>> fax:     +39 0584 1660272
>>>>>>>> mob:   +39 333 8128928
>>>>>>>>
>>>>>>>> http://www.geo-solutions.it
>>>>>>>> http://twitter.com/geosolutions_it
>>>>>>>>
>>>>>>>> -------------------------------------------------------
>>>>>>>> AVVERTENZE AI SENSI DEL D.Lgs. 196/2003
>>>>>>>> Le informazioni contenute in questo messaggio di posta elettronica
>>>>>>>> e/o
>>>>>>>> nel/i file/s allegato/i sono da considerarsi strettamente riservate.
>>>>>>>> Il loro utilizzo è consentito esclusivamente al destinatario del
>>>>>>>> messaggio, per le finalità indicate nel messaggio stesso. Qualora
>>>>>>>> riceviate questo messaggio senza esserne il destinatario, Vi
>>>>>>>> preghiamo
>>>>>>>> cortesemente di darcene notizia via e-mail e di procedere alla
>>>>>>>> distruzione del messaggio stesso, cancellandolo dal Vostro sistema.
>>>>>>>> Conservare il messaggio stesso, divulgarlo anche in parte,
>>>>>>>> distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per
>>>>>>>> finalità
>>>>>>>> diverse, costituisce comportamento contrario ai principi dettati dal
>>>>>>>> D.Lgs. 196/2003.
>>>>>>>>
>>>>>>>> The information in this message and/or attachments, is intended
>>>>>>>> solely
>>>>>>>> for the attention and use of the named addressee(s) and may be
>>>>>>>> confidential or proprietary in nature or covered by the provisions
>>>>>>>> of
>>>>>>>> privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
>>>>>>>> Data Protection Code).Any use not in accord with its purpose, any
>>>>>>>> disclosure, reproduction, copying, distribution, or either
>>>>>>>> dissemination, either whole or partial, is strictly forbidden except
>>>>>>>> previous formal approval of the named addressee(s). If you are not
>>>>>>>> the
>>>>>>>> intended recipient, please contact immediately the sender by
>>>>>>>> telephone, fax or e-mail and delete the information in this message
>>>>>>>> that has been received in error. The sender does not give any
>>>>>>>> warranty
>>>>>>>> or accept liability as the content, accuracy or completeness of sent
>>>>>>>> messages and accepts no responsibility  for changes made after they
>>>>>>>> were sent or for other risks which arise as a result of e-mail
>>>>>>>> transmission, viruses, etc.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Jun 14, 2016 at 8:09 PM, Devon Tucker
>>>>>>>> <devonrtuc...@gmail.com> wrote:
>>>>>>>> > Hi all,
>>>>>>>> >
>>>>>>>> > After discussions about the ImageMosaic API proposal we have
>>>>>>>> > decided to
>>>>>>>> > break it up into a few smaller pieces that are hopefully both more
>>>>>>>> > manageable to implement and easier to understand. First up is a
>>>>>>>> > proposal to
>>>>>>>> > refactor the ImageMosaic CatalogManager to allow parts of it to be
>>>>>>>> > overridden. The first new proposal is here:
>>>>>>>> >
>>>>>>>> >
>>>>>>>> > https://github.com/geotools/geotools/wiki/Refactor-ImageMosaic-Index-and-Catalog-management-for-improved-extensibility
>>>>>>>> >
>>>>>>>> > I've been doing the work on my own branch here:
>>>>>>>> >
>>>>>>>> > https://github.com/dvntucker/geotools/tree/im_api_refactor
>>>>>>>> >
>>>>>>>> > Further to that, I have another branch where I've been storing R&D
>>>>>>>> > work done
>>>>>>>> > against that branch. Most of that work was done much earlier, but
>>>>>>>> > I've been
>>>>>>>> > porting my previous test cases to the new branch as I go along for
>>>>>>>> > verification purposes.
>>>>>>>> >
>>>>>>>> > Please take a look and let me know if there's anything that isn't
>>>>>>>> > clear.
>>>>>>>> > This proposal is much simpler than the previous one.
>>>>>>>> >
>>>>>>>> > Cheers
>>>>>>>> >
>>>>>>>> >
>>>>>>>> > ------------------------------------------------------------------------------
>>>>>>>> > What NetFlow Analyzer can do for you? Monitors network bandwidth
>>>>>>>> > and traffic
>>>>>>>> > patterns at an interface-level. Reveals which users, apps, and
>>>>>>>> > protocols are
>>>>>>>> > consuming the most bandwidth. Provides multi-vendor support for
>>>>>>>> > NetFlow,
>>>>>>>> > J-Flow, sFlow and other flows. Make informed decisions using
>>>>>>>> > capacity
>>>>>>>> > planning
>>>>>>>> > reports.
>>>>>>>> >
>>>>>>>> > http://pubads.g.doubleclick.net/gampad/clk?id=1444514421&iu=/41014381
>>>>>>>> > _______________________________________________
>>>>>>>> > GeoTools-Devel mailing list
>>>>>>>> > GeoTools-Devel@lists.sourceforge.net
>>>>>>>> > https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>>>>>> >
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ------------------------------------------------------------------------------
>>>>>>> What NetFlow Analyzer can do for you? Monitors network bandwidth and
>>>>>>> traffic
>>>>>>> patterns at an interface-level. Reveals which users, apps, and
>>>>>>> protocols are
>>>>>>> consuming the most bandwidth. Provides multi-vendor support for
>>>>>>> NetFlow,
>>>>>>> J-Flow, sFlow and other flows. Make informed decisions using capacity
>>>>>>> planning
>>>>>>> reports. http://sdm.link/zohomanageengine
>>>>>>> _______________________________________________
>>>>>>> GeoTools-Devel mailing list
>>>>>>> GeoTools-Devel@lists.sourceforge.net
>>>>>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to