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 spotify docker-client to 3.2.1 #693

Merged
merged 1 commit into from Nov 11, 2015
Merged

Conversation

gbougeard
Copy link
Contributor

Using the spotify docker client in Play 2.4.x projects leads to jackson errors like :
[error] (docker:publishLocal) com.spotify.docker.client.DockerException: java.util.concurrent.ExecutionException: javax.ws.rs.ProcessingException: java.lang.AbstractMethodError: com.fasterxml.jackson.jaxrs.base.ProviderBase._configForReading(Lcom/fasterxml/jackson/databind/ObjectReader;[Ljava/lang/annotation/Annotation;)Lcom/fasterxml/jackson/jaxrs/cfg/EndpointConfigBase;

Upgrading to 3.2.1 fixes it.

Using the spotify docker client in Play 2.4.x projects leads to jackson errors like :
```[error] (docker:publishLocal) com.spotify.docker.client.DockerException: java.util.concurrent.ExecutionException: javax.ws.rs.ProcessingException: java.lang.AbstractMethodError: com.fasterxml.jackson.jaxrs.base.ProviderBase._configForReading(Lcom/fasterxml/jackson/databind/ObjectReader;[Ljava/lang/annotation/Annotation;)Lcom/fasterxml/jackson/jaxrs/cfg/EndpointConfigBase;```

Upgrading to 3.2.1 fixes it.
@muuki88 muuki88 added the docker label Nov 11, 2015
muuki88 added a commit that referenced this pull request Nov 11, 2015
Upgrade spotify docker-client to 3.2.1
@muuki88 muuki88 merged commit d10beef into sbt:master Nov 11, 2015
@muuki88
Copy link
Contributor

muuki88 commented Nov 11, 2015

Thanks a lot for keeping an eye on that. At some point we made refactor these modules out into subpackages to avoid that kind of issues (JDeb, DockerSpotifiy,... )

@huntc
Copy link
Contributor

huntc commented Dec 18, 2015

@muuki88 It appears as though this PR has brought in a ton of new dependencies on glassfish artifacts via Spotify's docker-client. I waited considerably longer for the latest native packager to download (1.0.6).

Do we really need the spotify client? Should we consider an alternative?

When we require docker communication then we just use the process API. Thoughts?

@huntc
Copy link
Contributor

huntc commented Dec 18, 2015

Perhaps it is also time to consider factoring out Docker into its own plugin. It ain't the only container format. ;-)

@muuki88
Copy link
Contributor

muuki88 commented Dec 19, 2015

Yeah, I actually planned to factor out the JDeb and Spotify support in it's own plugin which is bound to the release cycle of the sbt-native-packager. I will try to upgrade our build infra today

@huntc
Copy link
Contributor

huntc commented Dec 19, 2015

FWIW I think jdeb is useful to support building Debian packages in general, and given that Debian is a popular target I'd leave it in there. Docker should absolutely be standalone though.

Separately, we should probably avoid the Spotify client for Docker given what it pulls in and just integrate at the process API level.

WDYT?

@muuki88
Copy link
Contributor

muuki88 commented Dec 20, 2015

Keeping jdeb in the core for the first refactoring is fine with me. However we had some issue with jdeb as well, because it was depending on maven2 which introduced a lot of unwanted logging dependencies. So in the long run I would like to factor this out as well.

just integrate at the process API level

What do you mean by that? The native-docker plugin integrates the docker command. The spotify-docker client is like jdeb a java-based abstraction, which allows native-packager to run on platforms that don't have the docker (or dpkg) command available. So this is in general the same thing.

Yesterday I simplified our build, reusing a lot of sbt-plugins out there. My idea would be to separate the build like this

lazy val root = project.aggregate(core, dockerSpotify)

lazy val core = project.in(file("core"))
  .settings(
    name := "sbt-native-packager"
)

lazy val dockerSpotify = project.in(file("docker-spotify"))
  .settings(
     name := "sbt-native-packager-docker-spotify"
  )
  .dependsOn(core % "compile->provided") // not quite sure if this works

So that in the end you write in your plugins.sbt

val nativePackagerVersion = "1.1.0"
addSbtPlugin("com.typesafe.sbt", "sbt-native-packager", nativePackagerVersion)
addSbtPlugin("com.typesafe.sbt", "sbt-native-packager-docker-spotify", nativePackagerVersion)

@muuki88
Copy link
Contributor

muuki88 commented Dec 21, 2015

Actually, I have a better idea. That it's a lot easier. Marking the dependencies with % "provided".
We can then make Class.forName checks in the plugin using these to output better errors messages and the user just needs to add the specific library in the plugins.sbt

@huntc
Copy link
Contributor

huntc commented Dec 21, 2015

Yeah, I think that could work, although I still think that Docker should be an entirely separate plugin ultimately. :-)

On 22 Dec 2015, at 5:21 AM, Nepomuk Seiler notifications@github.com wrote:

Actually, I have a better idea. That it's a lot easier. Marking the dependencies with % "provided".
We can then make Class.forName checks in the plugin using these to output better errors messages and the user just needs to add the specific library in the plugins.sbt


Reply to this email directly or view it on GitHub.

@muuki88
Copy link
Contributor

muuki88 commented Dec 24, 2015

I think we should keep it. We only should avoid the dependency mess. There is another sbt-docker plugin, which is pretty nice. However you have to configure everything for your application by yourself.

@huntc
Copy link
Contributor

huntc commented Dec 24, 2015

Sounds like a plan. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants