New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rest API Conversion to Spring MVC with API Documentation [GEOS-7931] #2216

Merged
merged 310 commits into from Apr 11, 2017

Conversation

Projects
None yet
@tbarsballe
Member

tbarsballe commented Apr 8, 2017

ianturton and others added some commits Mar 28, 2017

@jodygarnett

This comment has been minimized.

Show comment
Hide comment
@jodygarnett

jodygarnett Apr 8, 2017

Member

Woo hoo!

Member

jodygarnett commented Apr 8, 2017

Woo hoo!

@bencaradocdavies

This comment has been minimized.

Show comment
Hide comment
@bencaradocdavies

bencaradocdavies Apr 8, 2017

Member

Do not merge this pull request. There is still an intermittent failure in gs-h2 RestTest:

createDataStoreUsingRestSingleFile(org.geoserver.h2.RestTest)  Time elapsed: 267 sec  <<< FAILURE!
java.lang.AssertionError: 
Expected: is <201>
     but: was <202>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.junit.Assert.assertThat(Assert.java:865)
	at org.junit.Assert.assertThat(Assert.java:832)
	at org.geoserver.h2.RestTest.genericCreateDataStoreUsingRestTest(RestTest.java:123)
	at org.geoserver.h2.RestTest.createDataStoreUsingRestSingleFile(RestTest.java:105)

Despite REST URL with ?configure=all, sometimes source.getNames() is empty at line 412 in DataStoreFileController.dataStorePut.

Member

bencaradocdavies commented Apr 8, 2017

Do not merge this pull request. There is still an intermittent failure in gs-h2 RestTest:

createDataStoreUsingRestSingleFile(org.geoserver.h2.RestTest)  Time elapsed: 267 sec  <<< FAILURE!
java.lang.AssertionError: 
Expected: is <201>
     but: was <202>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.junit.Assert.assertThat(Assert.java:865)
	at org.junit.Assert.assertThat(Assert.java:832)
	at org.geoserver.h2.RestTest.genericCreateDataStoreUsingRestTest(RestTest.java:123)
	at org.geoserver.h2.RestTest.createDataStoreUsingRestSingleFile(RestTest.java:105)

Despite REST URL with ?configure=all, sometimes source.getNames() is empty at line 412 in DataStoreFileController.dataStorePut.

@bencaradocdavies

This comment has been minimized.

Show comment
Hide comment
@bencaradocdavies

bencaradocdavies Apr 8, 2017

Member

Many modules in the communityRelease profile do not compile. This PR cannot be merged until these modules compile.

Member

bencaradocdavies commented Apr 8, 2017

Many modules in the communityRelease profile do not compile. This PR cannot be merged until these modules compile.

@aaime

This comment has been minimized.

Show comment
Hide comment
@aaime

aaime Apr 8, 2017

Member

@bencaradocdavies the idea during the sprint was to kick them out of the build (them being community modules, and thus unsupported). I'd still go on the ml and warn people providing a list of the modules that will be removed.

Member

aaime commented Apr 8, 2017

@bencaradocdavies the idea during the sprint was to kick them out of the build (them being community modules, and thus unsupported). I'd still go on the ml and warn people providing a list of the modules that will be removed.

@bencaradocdavies

This comment has been minimized.

Show comment
Hide comment
@bencaradocdavies

bencaradocdavies Apr 8, 2017

Member

@aaime that sounds good. Removal from the community profile should provide incentive to the maintainers to migrate to the new REST implementation. Some of the fixes might be just package name changes, which was all that I had to do for app-schema.

Member

bencaradocdavies commented Apr 8, 2017

@aaime that sounds good. Removal from the community profile should provide incentive to the maintainers to migrate to the new REST implementation. Some of the fixes might be just package name changes, which was all that I had to do for app-schema.

@jodygarnett

This comment has been minimized.

Show comment
Hide comment
@jodygarnett

jodygarnett Apr 10, 2017

Member

Do we have a Jira issue for this? Or many Jira issues ...

Member

jodygarnett commented Apr 10, 2017

Do we have a Jira issue for this? Or many Jira issues ...

@jodygarnett

This comment has been minimized.

Show comment
Hide comment
@jodygarnett

jodygarnett Apr 10, 2017

Member

I am really looking forward to getting this in! Thanks for all the hard work everyone. Also to everyone who worked post-sprint: Andrea (passing tests) and Torben (finishing importer and making the code consistent for this PR.

Member

jodygarnett commented Apr 10, 2017

I am really looking forward to getting this in! Thanks for all the hard work everyone. Also to everyone who worked post-sprint: Andrea (passing tests) and Torben (finishing importer and making the code consistent for this PR.

@tbarsballe

This comment has been minimized.

Show comment
Hide comment
@tbarsballe

tbarsballe Apr 10, 2017

Member

Do we have a Jira issue for this? Or many Jira issues ...

Good question. As far as I can tell, there is neither a JIRA issue nor a GSIP. This is mostly just documented through wiki pages and mailing list discussion.

Looking back to last years Wicket migration, I don't think we had an JIRA issue for that either.

Member

tbarsballe commented Apr 10, 2017

Do we have a Jira issue for this? Or many Jira issues ...

Good question. As far as I can tell, there is neither a JIRA issue nor a GSIP. This is mostly just documented through wiki pages and mailing list discussion.

Looking back to last years Wicket migration, I don't think we had an JIRA issue for that either.

@jodygarnett jodygarnett changed the title from Rest API Conversion from Restlet to Spring MVC to Rest API Conversion to Spring MVC with API Documentation [GEOS-7931] Apr 10, 2017

@jodygarnett

One minor rename/typo.

<goal>generate</goal>
</goals>
<configuration>
<inputSpec>src/main/resources/api/coverages.yaml</inputSpec>

This comment has been minimized.

@jodygarnett

jodygarnett Apr 11, 2017

Member

This swagger-codegen-maven-plugin is terrible, it should be able to match *.yaml.

@jodygarnett

jodygarnett Apr 11, 2017

Member

This swagger-codegen-maven-plugin is terrible, it should be able to match *.yaml.

@@ -0,0 +1,186 @@
---

This comment has been minimized.

@jodygarnett

jodygarnett Apr 11, 2017

Member

does swagger have comments? for osgeo (c) 2017

@jodygarnett

jodygarnett Apr 11, 2017

Member

does swagger have comments? for osgeo (c) 2017

This comment has been minimized.

@tbarsballe

tbarsballe Apr 11, 2017

Member

Comments in yaml use "#" I believe

@tbarsballe

tbarsballe Apr 11, 2017

Member

Comments in yaml use "#" I believe

Show outdated Hide outdated ...ig/src/main/java/org/geoserver/rest/catalog/CoverageStoreController.java
}
@DeleteMapping(value = "{storeName}")
public void overageStoreDelete(

This comment has been minimized.

@jodygarnett

jodygarnett Apr 11, 2017

Member

Rename from {{overageStoreDelete}} to {{coverageStoreDelete}}

@jodygarnett

jodygarnett Apr 11, 2017

Member

Rename from {{overageStoreDelete}} to {{coverageStoreDelete}}

This comment has been minimized.

@jodygarnett

jodygarnett Apr 11, 2017

Member

Fixed

@jodygarnett
@jodygarnett

This comment has been minimized.

Show comment
Hide comment
@jodygarnett

jodygarnett Apr 11, 2017

Member

From conversation with @bencaradocdavies - the H2 failure has occurred on master before:

geoserver-master 4475 is the only one that confirms the failure I saw on this branch (createDataStoreUsingRestSingleFile), but it is confirmed on master, not a new regression, and thus not a blocker for merge.

Member

jodygarnett commented Apr 11, 2017

From conversation with @bencaradocdavies - the H2 failure has occurred on master before:

geoserver-master 4475 is the only one that confirms the failure I saw on this branch (createDataStoreUsingRestSingleFile), but it is confirmed on master, not a new regression, and thus not a blocker for merge.

@bencaradocdavies

This comment has been minimized.

Show comment
Hide comment
@bencaradocdavies

bencaradocdavies Apr 11, 2017

Member

Kicked the PR build as geogig is back on master. This will give youse a full laundry list of community modules that need fixing or kicking.

Member

bencaradocdavies commented Apr 11, 2017

Kicked the PR build as geogig is back on master. This will give youse a full laundry list of community modules that need fixing or kicking.

@tbarsballe

This comment has been minimized.

Show comment
Hide comment
@tbarsballe

tbarsballe Apr 11, 2017

Member

After the latest CI build, the following community modules are failing:

[INFO] Core Scripting Extension ........................... FAILURE [  2.506 s]
[INFO] REST plugin community module ....................... FAILURE [  3.774 s]
[INFO] GeoServer JMS clustering module .................... FAILURE [  5.283 s]
[INFO] Resumable REST Upload module ....................... FAILURE [  1.903 s]
[INFO] GeoFence security integration ...................... FAILURE [  3.659 s]
[INFO] REST SLD service ................................... FAILURE [  2.318 s]
[INFO] GeoGig GeoServer integration ....................... FAILURE [ 19.913 s]
[INFO] GeoServer Config Backup And Restore REST Module .... FAILURE [  1.894 s]
[INFO] GeoServer notification event system ................ FAILURE [  4.166 s]
Member

tbarsballe commented Apr 11, 2017

After the latest CI build, the following community modules are failing:

[INFO] Core Scripting Extension ........................... FAILURE [  2.506 s]
[INFO] REST plugin community module ....................... FAILURE [  3.774 s]
[INFO] GeoServer JMS clustering module .................... FAILURE [  5.283 s]
[INFO] Resumable REST Upload module ....................... FAILURE [  1.903 s]
[INFO] GeoFence security integration ...................... FAILURE [  3.659 s]
[INFO] REST SLD service ................................... FAILURE [  2.318 s]
[INFO] GeoGig GeoServer integration ....................... FAILURE [ 19.913 s]
[INFO] GeoServer Config Backup And Restore REST Module .... FAILURE [  1.894 s]
[INFO] GeoServer notification event system ................ FAILURE [  4.166 s]

@tbarsballe tbarsballe merged commit 3518cdb into master Apr 11, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment