Hi Andrea,
I took the new process selection stuff for a spin today and noticed some issues. I have a pull request with some fixes.
https://github.com/geoserver/geoserver/pull/15
Here the individual changes in detail.
First issue was that i noticed that the spring factory was not loading. Cause was it was being registered after the initial initialization of the WPSInfo.getProcessGroups() property. I think this change fixes the current build failures as well.
https://github.com/jdeolive/geoserver/commit/2509c8ca2da39d01ede7579c2f79c9e7dc796c6d
Second issue was that no initialization of the WPSInfo.getProcessGroups() property was happening in cases whether wps configuration is created from scratch. I moved the initialization out from the bean object and into the loader which seems more inline with trying to keep Info objects as dumb beans.
https://github.com/jdeolive/geoserver/commit/d408809016eb902d5dc62d8f86ba829f28db4ee2
The rest is minor stuff.
Making WPSInfo property/setter consistent with getter.
https://github.com/jdeolive/geoserver/commit/b3b1be84846a2e5a07b371a83352e7ebb253fd60
Dropping “info” suffix from xstream representation which we generally don’t include with the other Info object aliases.
https://github.com/jdeolive/geoserver/commit/d09378fa27a5a71705839df738bd41621be455d0
-Justin
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
On Mon, Aug 13, 2012 at 4:31 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:
Hi Andrea,
I took the new process selection stuff for a spin today and noticed some issues. I have a pull request with some fixes.
https://github.com/geoserver/geoserver/pull/15
Here the individual changes in detail.
First issue was that i noticed that the spring factory was not loading. Cause was it was being registered after the initial initialization of the WPSInfo.getProcessGroups() property. I think this change fixes the current build failures as well.
https://github.com/jdeolive/geoserver/commit/2509c8ca2da39d01ede7579c2f79c9e7dc796c6d
Second issue was that no initialization of the WPSInfo.getProcessGroups() property was happening in cases whether wps configuration is created from scratch. I moved the initialization out from the bean object and into the loader which seems more inline with trying to keep Info objects as dumb beans.
https://github.com/jdeolive/geoserver/commit/d408809016eb902d5dc62d8f86ba829f28db4ee2
Forgot a commit for this one as well.
https://github.com/jdeolive/geoserver/commit/e6b13ace72dc60c05444cd07b12a67852b29e7ee
The rest is minor stuff.
Making WPSInfo property/setter consistent with getter.
https://github.com/jdeolive/geoserver/commit/b3b1be84846a2e5a07b371a83352e7ebb253fd60
Dropping “info” suffix from xstream representation which we generally don’t include with the other Info object aliases.
https://github.com/jdeolive/geoserver/commit/d09378fa27a5a71705839df738bd41621be455d0
-Justin
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
On Tue, Aug 14, 2012 at 12:31 AM, Justin Deoliveira <jdeolive@anonymised.com> wrote:
Hi Andrea,
I took the new process selection stuff for a spin today and noticed some issues. I have a pull request with some fixes.
https://github.com/geoserver/geoserver/pull/15
Here the individual changes in detail.
First issue was that i noticed that the spring factory was not loading. Cause was it was being registered after the initial initialization of the WPSInfo.getProcessGroups() property. I think this change fixes the current build failures as well.
https://github.com/jdeolive/geoserver/commit/2509c8ca2da39d01ede7579c2f79c9e7dc796c6d
That’s odd, there is no build failure (the one from yesterday was due to the UI frontend changes).
Looking at the changes, I believe this might be due to a different filesystem layout that makes
Spring load the beans in a different order.
The change is fine by me.
Second issue was that no initialization of the WPSInfo.getProcessGroups() property was happening in cases whether wps configuration is created from scratch. I moved the initialization out from the bean object and into the loader which seems more inline with trying to keep Info objects as dumb beans.
https://github.com/jdeolive/geoserver/commit/d408809016eb902d5dc62d8f86ba829f28db4ee2
Hmm… that’s problematic as well for two reasons:
- that initialization won’t happen for people upgrading from 1.7 (people upgrading from 1.7 with WPS? As incredible as it may sound, I’ve seen a couple)
- the initialization might happen too soon and miss factories that register programmatically on startup (all the spring based ones do),
with the result that the GUI won’t offer a way to configure those
Maybe we should move it to a GeoServer initializer or a lifecicle listerer?
That way we could have a way to also update the configuration on a “reset” and 2reload" and add
feature factories that might have popped up in the meantime
Cheers
Andrea
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 962313
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
On Tue, Aug 14, 2012 at 2:34 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:
On Tue, Aug 14, 2012 at 12:31 AM, Justin Deoliveira <jdeolive@anonymised.com> wrote:
Hi Andrea,
I took the new process selection stuff for a spin today and noticed some issues. I have a pull request with some fixes.
https://github.com/geoserver/geoserver/pull/15
Here the individual changes in detail.
First issue was that i noticed that the spring factory was not loading. Cause was it was being registered after the initial initialization of the WPSInfo.getProcessGroups() property. I think this change fixes the current build failures as well.
https://github.com/jdeolive/geoserver/commit/2509c8ca2da39d01ede7579c2f79c9e7dc796c6d
That’s odd, there is no build failure (the one from yesterday was due to the UI frontend changes).
Looking at the changes, I believe this might be due to a different filesystem layout that makes
Spring load the beans in a different order.
The change is fine by me.
It was just a guess, looks like it wasn’t the case but Ben sorted it out.
Second issue was that no initialization of the WPSInfo.getProcessGroups() property was happening in cases whether wps configuration is created from scratch. I moved the initialization out from the bean object and into the loader which seems more inline with trying to keep Info objects as dumb beans.
https://github.com/jdeolive/geoserver/commit/d408809016eb902d5dc62d8f86ba829f28db4ee2
Hmm… that’s problematic as well for two reasons:
- that initialization won’t happen for people upgrading from 1.7 (people upgrading from 1.7 with WPS? As incredible as it may sound, I’ve seen a couple)
- the initialization might happen too soon and miss factories that register programmatically on startup (all the spring based ones do),
with the result that the GUI won’t offer a way to configure those
Maybe we should move it to a GeoServer initializer or a lifecicle listerer?
That way we could have a way to also update the configuration on a “reset” and 2reload" and add
feature factories that might have popped up in the meantime
Works for me. There already is a WPSinitializer class so we could potentially move it there.
As for picking up new factories i am not sure how that would work since the process groups are only added if the WPSInfo.getProcessGroups() property is null… if it has a value we leave it as is right? Maybe we could add something to ui which would basically be a reset that would clear the current groups and reinitialize from all found.
Cheers
Andrea
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 962313
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
On Tue, Aug 14, 2012 at 3:47 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:
That’s odd, there is no build failure (the one from yesterday was due to the UI frontend changes).
Looking at the changes, I believe this might be due to a different filesystem layout that makes
Spring load the beans in a different order.
The change is fine by me.
It was just a guess, looks like it wasn’t the case but Ben sorted it out.
Ah right, Martin’s changes to the vector process metadata.
Second issue was that no initialization of the WPSInfo.getProcessGroups() property was happening in cases whether wps configuration is created from scratch. I moved the initialization out from the bean object and into the loader which seems more inline with trying to keep Info objects as dumb beans.
https://github.com/jdeolive/geoserver/commit/d408809016eb902d5dc62d8f86ba829f28db4ee2
Hmm… that’s problematic as well for two reasons:
- that initialization won’t happen for people upgrading from 1.7 (people upgrading from 1.7 with WPS? As incredible as it may sound, I’ve seen a couple)
- the initialization might happen too soon and miss factories that register programmatically on startup (all the spring based ones do),
with the result that the GUI won’t offer a way to configure those
Maybe we should move it to a GeoServer initializer or a lifecicle listerer?
That way we could have a way to also update the configuration on a “reset” and 2reload" and add
feature factories that might have popped up in the meantime
Works for me. There already is a WPSinitializer class so we could potentially move it there.
As for picking up new factories i am not sure how that would work since the process groups are only added if the WPSInfo.getProcessGroups() property is null… if it has a value we leave it as is right? Maybe we could add something to ui which would basically be a reset that would clear the current groups and reinitialize from all found.
What we could have is a GeoServerLifecycle listenter that’s somehow tightly coupled with WPSInfo, on a reset or reload call
it would take the existing process groups, compare with factories coming from Processors, and add the missing ones.
Cheers
Andrea
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 962313
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
On Tue, Aug 14, 2012 at 8:25 AM, Andrea Aime <andrea.aime@anonymised.com> wrote:
On Tue, Aug 14, 2012 at 3:47 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:
That’s odd, there is no build failure (the one from yesterday was due to the UI frontend changes).
Looking at the changes, I believe this might be due to a different filesystem layout that makes
Spring load the beans in a different order.
The change is fine by me.
It was just a guess, looks like it wasn’t the case but Ben sorted it out.
Ah right, Martin’s changes to the vector process metadata.
Second issue was that no initialization of the WPSInfo.getProcessGroups() property was happening in cases whether wps configuration is created from scratch. I moved the initialization out from the bean object and into the loader which seems more inline with trying to keep Info objects as dumb beans.
https://github.com/jdeolive/geoserver/commit/d408809016eb902d5dc62d8f86ba829f28db4ee2
Hmm… that’s problematic as well for two reasons:
- that initialization won’t happen for people upgrading from 1.7 (people upgrading from 1.7 with WPS? As incredible as it may sound, I’ve seen a couple)
- the initialization might happen too soon and miss factories that register programmatically on startup (all the spring based ones do),
with the result that the GUI won’t offer a way to configure those
Maybe we should move it to a GeoServer initializer or a lifecicle listerer?
That way we could have a way to also update the configuration on a “reset” and 2reload" and add
feature factories that might have popped up in the meantime
Works for me. There already is a WPSinitializer class so we could potentially move it there.
As for picking up new factories i am not sure how that would work since the process groups are only added if the WPSInfo.getProcessGroups() property is null… if it has a value we leave it as is right? Maybe we could add something to ui which would basically be a reset that would clear the current groups and reinitialize from all found.
What we could have is a GeoServerLifecycle listenter that’s somehow tightly coupled with WPSInfo, on a reset or reload call
it would take the existing process groups, compare with factories coming from Processors, and add the missing ones.
Ahh ok, i thought that the lack of a process group info object meant it wsa disabled, but there is an explicit flag for that cool. Ok, so updated the pull request with this change.
https://github.com/jdeolive/geoserver/commit/7ba804e07340aabc40cf65c791130f6cf459d08b
And have tested that the upgrade works post and pre 2.0.x (ie 1.7.x upgrade) and also verified a reload works. Ok to merge?
Cheers
Andrea
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 962313
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it
–
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.
On Tue, Aug 14, 2012 at 5:48 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:
What we could have is a GeoServerLifecycle listenter that’s somehow tightly coupled with WPSInfo, on a reset or reload call
it would take the existing process groups, compare with factories coming from Processors, and add the missing ones.
Ahh ok, i thought that the lack of a process group info object meant it wsa disabled, but there is an explicit flag for that cool. Ok, so updated the pull request with this change.
https://github.com/jdeolive/geoserver/commit/7ba804e07340aabc40cf65c791130f6cf459d08b
And have tested that the upgrade works post and pre 2.0.x (ie 1.7.x upgrade) and also verified a reload works. Ok to merge?
Yep thanks!
Cheers
Andrea
–
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 962313
mob: +39 339 8844549
http://www.geo-solutions.it
http://twitter.com/geosolutions_it