Skip to content
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

Upgrade to pom-scijava 29 #42

Merged
merged 4 commits into from
Aug 16, 2020
Merged

Upgrade to pom-scijava 29 #42

merged 4 commits into from
Aug 16, 2020

Conversation

frauzufall
Copy link
Member

@frauzufall frauzufall commented Aug 13, 2020

This PR depends on the following PRs:

Outline of the changes:

  • upgrade pom-scijava to 29.2.1
  • upgrade dropwizard from 1.2.0 to 2.0.9
  • upgrade jersey-media-multipart from 2.23.2 to 2.31
  • use jackson version 2.11.2
  • delete DefaultTableIOPlugin, use TableIOService instead
  • make TableIOPluginTest use new options API for reading and writing tables
  • use BytesHandle instead of ByteArrayHandle

All tests run successfully for me now with the branches from the other PRs, but since I never used imagej-server for anything, it'd be great to have a real world tester :) And of course, all of the above PRs first need to be reviewed before this can be finished.

Here are the JARs if anyone wants to give it a try:
imagej-server-pom-sj-29.zip

@imagejan
Copy link
Member

@frauzufall with new releases of scijava-common-2.84.0, scijava-table-0.7.0 and scijava-plugins-io-table-0.4.0, this pull request should be good to go now, right?

@frauzufall
Copy link
Member Author

@imagejan I will update the versions and let the tests speak :) Thank you so much for reviewing and releasing all of this!

- upgrade pom-scijava to 29.2.1
- upgrade dropwizard from 1.2.0 to 2.0.9
- upgrade jersey-media-multipart from 2.23.2 to 2.31
- use jackson version 2.11.2
- delete DefaultTableIOPlugin, use TableIOService instead
- make TableIOPluginTest use new options API for reading and writing
tables
- use BytesHandle instead of ByteArrayHandle
@frauzufall frauzufall marked this pull request as ready for review August 14, 2020 10:16
@ctrueden
Copy link
Member

ctrueden commented Aug 16, 2020

I tested all the main APIs documented in the README. Some of the curl commands there were outdated, so I updated them with a0b9037.

The good news: All the I/O-related endpoints still work! 🎉 And the SciJava module-related endpoints work too! 🎉

The bad news: The /admin/stop endpoint seems to be broken:

curl -XDELETE localhost:8080/admin/stop

Returns nothing. But the server does not stop, and on the server side we see:

Exception in thread "Thread-34" java.lang.NoSuchMethodError: net.imagej.server.resources.AdminResource.access$0(Lnet/imagej/server/resources/AdminResource;)Lio/dropwizard/setup/Environment;
	at net.imagej.server.resources.AdminResource$1.run(AdminResource.java:57)
	at java.lang.Thread.run(Thread.java:748)
0:0:0:0:0:0:0:1 - - [16/Aug/2020:17:04:40 +0000] "DELETE /admin/stop HTTP/1.1" 200 0 "-" "curl/7.54.0" 1

It looks to me like some version skew across Dropwizard dependencies. @frauzufall I noticed that you stopped using the dropwizard-testing dependency management block in favor of manually specified versions. However, it seems there is a dropwizard-bom specifically for dependency management—I'll try using that instead and see whether it clears things up.

This makes the /admin/stop endpoint functional again.
@ctrueden
Copy link
Member

After 69d73f5, I was able to use /admin/stop successfully. However, in further testing while developing some integration tests, I once again saw the NoSuchMethodError. Strange. And now trying again to reproduce in a freshly started server, it works. So maybe the problem still exists, maybe not. But anyway, it's good enough for now! Will merge as soon as Travis gives the go-ahead.

@ctrueden ctrueden merged commit 37928d7 into master Aug 16, 2020
@ctrueden ctrueden deleted the pom-sj-29 branch August 16, 2020 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants