Re: [Geoserver-devel] (jdbc) resource store: platform additions

2016-01-06 Thread Jody Garnett
G'Day Niels, sorry it has taken me until the new year to return to this conversation. We discussed this pull request in the last geoserver meeting: * Andrea is creating a draft pull request review guide page to try and

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-23 Thread Niels Charlier
On 22-12-15 23:00, Jody Garnett wrote: > > Think you are missing the idea - in most cases these resources are not > unpacked on disk and are retrieved by JDBCResourceStore. I do not know > what that will do to performance - but that is why JDBCResourceStore > is a community module. If we need

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-22 Thread Jody Garnett
> The requirement is that if a resource listener is placed on a resource, > any change to that resource should be notified, also if that change > occurred in another instance of Geoserver in a clustered environment. Just > like the FileBasedResources notify all changes to a file. I would not >

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-22 Thread Niels Charlier
Hi Jody, The requirement is that if a resource listener is placed on a resource, any change to that resource should be notified, also if that change occurred in another instance of Geoserver in a clustered environment. Just like the FileBasedResources notify all changes to a file. I would not

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-21 Thread Jody Garnett
Thanks Neils, that approach matches my idea - I had in mind placing a listener on each style directory - and had hoped that we had relatively few other folders to track. I particular I was designing for a clustered environment and had hoped cluster workload could be sharded a bit by workspace (so

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-21 Thread Niels Charlier
Alright. What the clustering modules do is simply registering itself as a CatalogListener and then dispatch events to other geoserver instances. This is a fine strategy for the catalog, I don't see any reason to change it. The problem is that with resources, listeners are always placed on a

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-21 Thread Jody Garnett
Sounds excellent, thanks for your hard work. -- Jody Garnett On 21 December 2015 at 02:12, Niels Charlier wrote: > I'd suggest merging ResourceLoader and GeoServerDataDirectory. Both of > them provide mostly a lot of convenience methods and a little bit of extra > logic on top

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-21 Thread Ben Caradoc-Davies
Jody, have your questions been addressed to your satisfaction? Are we ready to merge PR #1361 as it is now? https://github.com/geoserver/geoserver/pull/1361 Kind regards, Ben. On 22/12/15 15:06, Jody Garnett wrote: > Sounds excellent, thanks for your hard work. -- Ben Caradoc-Davies

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-21 Thread Niels Charlier
I'd suggest merging ResourceLoader and GeoServerDataDirectory. Both of them provide mostly a lot of convenience methods and a little bit of extra logic on top of the ResourceStore. We can remove a lot of the depecrated stuff now. On 19-12-15 01:43, Jody Garnett wrote: And utility methods ...

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-21 Thread Jody Garnett
Looks like the methods have been addressed, still circling a bit on the event notification. What is your impression Niels? -- Jody Garnett On 21 December 2015 at 18:26, Ben Caradoc-Davies wrote: > Jody, > > have your questions been addressed to your satisfaction? Are we

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-18 Thread Niels Charlier
Hi Jody, Thanks for your review. On 17-12-15 00:36, Jody Garnett wrote: > - architecture reason: I was trying to have ResourceStore be the > single source of truth for resources (including change notification). > Is there a reason why this should change? Yes and no. There is a very clear

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-18 Thread Niels Charlier
On 18-12-15 16:04, Niels Charlier wrote: > >> - yeah I know you did not write this - just added one that takes a >> URL (do you really need both?) > > Yes, you do. The fromURL(String) method accepts things that would be > rejected by a normal URL parser, abolishing it would be a serious >

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-18 Thread Jody Garnett
And utility methods ... 2) New utility methods >> >> Resources.toURL( resource ) >> - makes a lot of sense and reads better in the code >> >> I am not too worried about additional utility methods, instead I am >> concerned about changes to objects we may have to implement (such as >>

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-18 Thread Jody Garnett
Comments inline on event distribution ... and then we should be good to go. If it helps I think we can seperate event notification (generating events/listening for events) from event distribution (in a cluster, recorded in an audit log). On 18 December 2015 at 07:04, Niels Charlier

Re: [Geoserver-devel] (jdbc) resource store: platform additions

2015-12-16 Thread Jody Garnett
Thanks Niels for taking this to the development list, I would like to ask you to bring these things up as they occur to you (rather than wait for the pull request review ) this will make your pull requests easier to review and more efficient for everyone here. Or you can work like Justin and

[Geoserver-devel] (jdbc) resource store: platform additions

2015-12-16 Thread Niels Charlier
Hi Everyone, We have a pull request for the jdbc resource store (as well as some additional resources port commits ) See https://github.com/geoserver/geoserver/pull/1361 It has been fully reviewed by Ben, but there is some new resources related stuff added to the platform module that I wanted