Get feedback from LinkedIn about the new plugin #77

pioterj · 2018-12-14T15:49:03Z

No description provided.

ymwang1984 · 2019-01-08T18:18:01Z

Hey, can LinkedIn leave comments in this ticket? Or should LinkedIn create another issue?

big-guy · 2019-01-08T19:13:43Z

I think feedback here would be good @ymwang1984

ymwang1984 · 2019-01-08T20:45:00Z

Unfortunately, the PoC from Play Team at the company left the team recently. So we are trying to sort out the replacement for now. Will have some update about it next week.

Right now, we are trying to enable Gradle 5 for Play apps. There are some sample LinkedIn apps that can run Gradle 5 now (not many, though). Within the mass Gradle 5 migration in the company, we are now prioritizing Play applications.

pioterj · 2019-01-09T10:58:43Z

@ymwang1984 The general feedback here would be great. Feel free to also open specific issues in this repo.

ymwang1984 · 2019-01-09T18:09:29Z

Thanks @pioterj

I have not tried the plugin on a sample app. Here are two questions I have in mind after reading the doc only (I could be very wrong though...):

(1) After the software model is gone (which is good for the long term), where will the existing setting go? Such as "custom twirl templates".

model { 
  components { 
    play { 
      sources { 
        withType(TwirlSourceSet) { 
          // use template format StreamFormat for all files named *.scala.stream 
          // additionally, include com.linkedin.sbt.streaming._, etc. imports in generated sources 
          addUserTemplateFormat('stream', 'com.linkedin.sbt.streaming.StreamFormat', 'com.linkedin.sbt.streaming._', 'com.linkedin.playplugins.streaming.StreamUtils._') 
        } 
      } 
    } 
  } 
}

(2) Is the hot reload mechanism changed? Do we need to add "--continuous" to have it now?

pioterj · 2019-01-10T12:30:22Z

@bmuschko Could you answer the questions above?

bmuschko · 2019-01-10T14:45:22Z

@ymwang1984 Find my answers below.

(1) After the software model is gone (which is good for the long term), where will the existing setting go? Such as "custom twirl templates".

In the "old" plugin this is only documented in the Javadocs of the SourceSet and not the user guide. Looks like we don't have the same Javadocs documentation for this plugin. I'll create an issue for it. The new SourceSet implementation is org.gradle.playframework.sourcesets.TwirlSourceSet. Basically you do it as such:

sourceSets {
    main {
        twirl {
            userTemplateFormats.add(newUserTemplateFormat("unused", "views.formats.unused.UnusedFormat"))
        }
    }
}

There's also a test case that demonstrates the usage.

(2) Is the hot reload mechanism changed? Do we need to add "--continuous" to have it now?

Continuous build is a cross-cutting concern. It will work the same as with the software model. You just add --continuous.

ymwang1984 · 2019-01-11T04:10:31Z

Thank you very much @bmuschko for the clear explanation.

Do we have some sample apps in the repo to test it? I recall the sample apps in Gradle repo was quite helpful in learning, debugging and verification.

bmuschko · 2019-01-11T14:36:56Z

@ymwang1984 You can find the samples here: https://github.com/gradle/playframework/tree/master/src/docs/samples. You can also find the link in the README.adoc.

pioterj · 2019-01-11T14:49:42Z

@ymwang1984 Note that you need to add the plugin version manually to make the samples work. See #76.

ymwang1984 · 2019-01-11T19:05:56Z

@pioterj and @bmuschko

Thank you! Very helpful information.

I tried one or two samples today briefly, I saw bunch of Test compilation failure in "compileTestScala". Looks like the dependency for Specs2 or Play test fixtures are missing in the samples. Could you confirm? I can do some hot reloads, though.

bmuschko · 2019-01-11T19:17:50Z

@ymwang1984 See #74. I believe they also fail in Gradle core right now. It's something we detected along the way.

ymwang1984 · 2019-01-11T20:10:36Z

Thanks @bmuschko so it is expected.

Is there ETA to fix that? Adding specs2 dependency still does not work for me locally.

It is not blocking since I will still be able to verify some other stuff. Thanks.

bmuschko · 2019-01-12T15:49:36Z

@ymwang1984 No ETA at the moment but I think soon. We were trying to gather as much feedback as possible to amount a big enough work package to keep a developer focused on the work for a couple of days at once. The more input you can give us the better.

ymwang1984 · 2019-01-16T16:48:57Z

Is this issue fixed? gradle/gradle#6789

We also had a support request ticket: https://support.gradle.com/hc/en-us/requests/1937

bmuschko · 2019-01-21T18:15:16Z

@ymwang1984 I think it's best to follow up on this in the support ticket.

ymwang1984 · 2019-01-22T16:57:09Z

Ok I will follow up with @big-guy on this.

bmuschko · 2019-01-29T00:11:50Z

@ymwang1984 We fixed the sample code (failing test) you were asking about before and added more Javadocs documentation. Did you have a chance to test the plugin with some other projects in the meantime?

ymwang1984 · 2019-01-29T03:53:32Z

@bmuschko

Thanks for the fix! I will try to verify some time later this week.

We are working on migrating Play applications to Gradle 5 at the moment, including the largest one (our Flagship product). They hit some issues and we are trying to resolve.

I am still curious about the solution of this bug fix request:
gradle/gradle#6789

It's still not entirely clear where the fix would go. Should it go to the new plugin? cc @big-guy

For LinkedIn, the issue is not really about the build time display. Multiple teams observed this behavior and would like to see the server up and running "the first time it is loaded". For a very large application, even if all steps are UP-TO-DATE the second round, it still adds quite some time to the development cycle. I'm happy to discuss with more details.

@garylin (manager of Play team at LinkedIn) may also want to open an issue about Gradle unable to support "-Dhttp.port=disabled"

Context:

https://www.playframework.com/documentation/2.6.x/ConfiguringHttps#Turning-HTTP-off
https://stackoverflow.com/questions/24864163/how-to-disable-http-port-in-play-framework

ymwang1984 · 2019-01-31T03:49:45Z

@bmuschko

Today one team hit the annotation processor problem when they attempted to onboard Gradle 5. In the end, we have a workaround. The issue was documented in

gradle/gradle#8378

The issue for this plugin is documented in

#84

bmuschko · 2019-02-06T20:38:34Z

@ymwang1984 You should be able to work around the issue in #84 in a similar fashion. See my comment.

bmuschko · 2019-02-19T14:08:58Z

@ymwang1984 Just letting you know that #84 has been fixed and a new version has been released. Did you have a chance to try out this plugin with one of your project?

ymwang1984 · 2019-02-20T20:37:53Z

@bmuschko Great news!

Currently we are still working on Gradle 5 upgrade for Play projects. Will update you once we have projects that try it with Lombok.

So I assume (with Gradle core) we will still use the workaround mentioned in gradle/gradle#8378 ?

bmuschko · 2019-02-20T22:49:22Z

@ymwang1984

Currently we are still working on Gradle 5 upgrade for Play projects. Will update you once we have projects that try it with Lombok.

OK, thanks.

So I assume (with Gradle core) we will still use the workaround mentioned in gradle/gradle#8378 ?

That's correct.

ymwang1984 · 2019-04-01T15:25:39Z

@bmuschko

Sorry for the late reply since our team is still working on Gradle 5 upgrade. But good news is that now we've got sufficient number of applications using Gradle 5.

We are testing it out with some of the apps. And we are hit with this slf4j binding problem: #90

Would appreciate if you could shed some light about how to work around this, thank you!

also cc @big-guy @pioterj

aaronlijie · 2019-04-02T17:31:51Z

This is Jie, working with @ymwang1984. @bmuschko The following is the stacktrace for our issue. It is pretty similiar with #90

SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/Users/jieli/.gradle/caches/modules-2/files-2.1/com.linkedin.spark-deployer-java/spark-deployer-java-impl/0.0.8/48c281be730e593d7cdfb74922162e37c37fedf5/spark-deployer-java-impl-0.0.8.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/Users/jieli/.gradle/ligradle/gradle-5.2.1/lib/gradle-logging-5.2.1.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [ch.qos.logback.classic.util.ContextSelectorStaticBinder]
java.lang.ExceptionInInitializerError
        at java.io.ObjectStreamClass.hasStaticInitializer(Native Method)
        at java.io.ObjectStreamClass.computeDefaultSUID(ObjectStreamClass.java:1787)
        at java.io.ObjectStreamClass.access$100(ObjectStreamClass.java:72)
        at java.io.ObjectStreamClass$1.run(ObjectStreamClass.java:253)
        at java.io.ObjectStreamClass$1.run(ObjectStreamClass.java:251)
        at java.security.AccessController.doPrivileged(Native Method)
        at java.io.ObjectStreamClass.getSerialVersionUID(ObjectStreamClass.java:250)
        at java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:611)
        at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1623)
        at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1518)
        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1774)
        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1351)
        at java.io.ObjectInputStream.defaultReadFields(ObjectInputStream.java:2000)
        at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:1924)
        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1801)
        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1351)
        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:371)
        at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:112)
        at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:68)
        at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:62)
        at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:67)
Caused by: java.lang.ClassCastException: ch.qos.logback.classic.Logger cannot be cast to org.gradle.api.logging.Logger
        at org.gradle.api.logging.Logging.getLogger(Logging.java:38)
        at org.gradle.playframework.tools.internal.run.PlayWorkerServer.<clinit>(PlayWorkerServer.java:22)
        ... 21 more

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':pemberly-skeleton-api-frontend:runPlay'.
> Process 'Gradle Play Worker 19' finished with non-zero exit value 1

bmuschko · 2019-04-02T18:01:40Z

@aaronlijie @ymwang1984 Thanks for reporting. Could you please put together a very simple project that reproduces the issue as requested here #90 (comment)? You likely have two different implementations of SLF4J on the classpath. My guess is that the worker API bleeds Gradle's dependencies into the classpath of the plugin. You can probably work around it by excluding the transitive SLF4J dependency.

aaronlijie · 2019-04-02T22:07:32Z

Hi
that spark-deployer-java-impl-0.0.8.jar is a fat jar which has org.slf4j.impl.xxx.
I have excluded spark-deployer-java-impl and logback. I confirmed that they were not in the build scan/ Dependencies. However, when I execute runPlay， The issue still existed:

SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/Users/jieli/.gradle/caches/modules-2/files-2.1/com.linkedin.spark-deployer-java/spark-deployer-java-impl/0.0.8/48c281be730e593d7cdfb74922162e37c37fedf5/spark-deployer-java-impl-0.0.8.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/Users/jieli/.gradle/ligradle/gradle-5.2.1/lib/gradle-logging-5.2.1.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [ch.qos.logback.classic.util.ContextSelectorStaticBinder]
java.lang.ExceptionInInitializerError

bmuschko · 2019-04-02T22:18:28Z

@aaronlijie Optimally, we'd trim down your example to minimal dependency that produces the issue. I have not seen the issue one some of the sample projects. Any chance you can put something together.

@big-guy Any advice you can give from the Gradle core side of things?

aaronlijie · 2019-04-02T22:51:09Z

I tried to reproduce it (add dependencies) in the sample projects, but I couldn't. And our project is coupled with linkedin's settings a lot and not easy to decouple.
Is it possible to fake classloader to let SLF4J bind to ch.qos.logback.classic.util.ContextSelectorStaticBinder?

Update: Let me updated something from my side.
spark-deployer-java-impl-0.0.8.jar is a fat jar which has org.slf4j.impl.xxx. I totally removed it from dependency and finally made the runPlay work. In the ticket #90,I think that swagger-codegen-cli is also a fat jar.
Hence I guess the issue is, if we have a fat jar which contains slf4j stuff, and that fat jar is loaded before other SLF4J, it will cause the issue.

aaronlijie · 2019-04-03T23:01:47Z

Here is a new issue:
We have another plugin which will do the following:

Set<File> sourceDirs = project.ideaModule.module.sourceDirs
sourceDirs.addAll(sourceSet.java.srcDirs)

It worked in the past and didn't work well with the new play gradle plugin. The stacktrace is

Caused by: org.gradle.api.internal.plugins.PluginApplicationException: Failed to apply plugin [id 'li-pegasus2']Open stacktrace
Caused by: java.lang.UnsupportedOperationException: (No message provided)Close stacktrace
at java_util_Set$addAll.call(Unknown Source)
at java_util_Set$addAll.call(Unknown Source)
at com.linkedin.pegasus.gradle.PegasusPlugin.addGeneratedDir(PegasusPlugin.groovy:868)
at com.linkedin.pegasus.gradle.PegasusPlugin.configureDataTemplateGeneration(PegasusPlugin.groovy:1357)
at org.gradle.internal.metaobject.BeanDynamicObject$MetaClassAdapter.invokeMethod(BeanDynamicObject.java:479)

I think the issue is related to:

playframework/src/main/java/org/gradle/playframework/plugins/PlayIdeaPlugin.java

Line 51 in 95e68fe

return Collections.unmodifiableSet(sourceDirs);

In the new plugin, it returns a unmodifiableSet. In the past I think the set is modifiable.

bmuschko · 2019-04-04T18:32:06Z

@aaronlijie I opened a pull request for your issue with the IDEA plugin: #91. Could you please create completely new GitHub issues if you encounter a problem and then link to them from here?

I will have to see if I can somehow reproduce your issue with SLF4J.

bmuschko · 2019-04-04T19:27:12Z

@aaronlijie Regarding the SLF4J issue I added a longer comment to #90 which applies to you as well. Sounds like you could work around it in the meantime.

bmuschko · 2019-04-10T18:14:47Z

@aaronlijie @ymwang1984 A fix for #91 has been release with version 0.5 of the plugin. Could you please give it a try?

mkurz · 2019-04-10T18:16:42Z

@bmuschko Can you push the v0.5 tag? https://github.com/gradle/playframework/releases

bmuschko · 2019-04-10T18:19:26Z

@mkurz done

aaronlijie · 2019-04-10T19:17:16Z

@aaronlijie @ymwang1984 A fix for #91 has been release with version 0.5 of the plugin. Could you please give it a try?

I have tried and the issue is fixed.

bmuschko · 2019-04-15T19:43:56Z

@aaronlijie @ymwang1984 Great, can you let us know as soon as you successfully adopted this plugin?

aaronlijie · 2019-04-17T22:41:10Z

@bmuschko
Hi
I created an separate #95 about " createXxxxxDistributionJar task behaves different under new playframework"

bmuschko · 2019-04-29T18:58:41Z

@aaronlijie A fix for #95 has been released with version 0.6 of the plugin.

bmuschko · 2019-05-05T13:35:21Z

@aaronlijie @ymwang1984 Is there anything left that needs to be addressed from our end until you can make a full transition to the new plugin?

aaronlijie · 2019-06-11T23:57:55Z

Hi
@bmuschko I create a issue about behavior change under new playFramework: #101.

Here is update from our side: we almost make our first project work with new playFramework, we plan to switch to new playFramework for that project in the following few days.

aaronlijie · 2019-07-19T02:20:32Z

Hi
@bmuschko I create a ticket about compileScala task. #109

It seems compilePlayRoutes task will make compileScala task always run instead of from_cache under some cases. Would you like to look at it ?

ymwang1984 · 2019-08-05T21:25:25Z

Hey @big-guy

Thanks for the release and the fix!

We noticed that it probably requires the latest Gradle 5.5. Is it true? Is it related to the latest fix in Worker API, etc?

We'd like the upgrade of this plugin to be decoupled from the upgrade of Gradle, if possible. It (coupling) may make migration more difficult for applications. Hope that makes sense :)

big-guy · 2019-08-06T12:57:20Z

@ymwang1984 I bumped the plugin build to 5.5.1, but I don't believe there's anything that ties the plugin to 5.5. I've been able to run the build with 5.4.1. I didn't go any older than that, but there have been few changes to the plugin since ~5.2.

aaronlijie · 2019-08-06T17:13:17Z

Hi @big-guy
Thank you for your reply.
I tested your release (playframework 0.7) with different gradle version in our project. The following is the result:

5.5.1 and 5.4.1 could work
5.2.1 couldn't work ( 5.2.1 is the version adopted at Linkedin)
the following is the error:

What went wrong:
A problem occurred evaluating project ':play-skeleton-frontend-on-gradle'.

Failed to apply plugin [class 'org.gradle.playframework.plugins.PlayApplicationPlugin']
Could not create task ':play-skeleton-frontend-on-gradle:runPlay'.
> Could not create task of type 'PlayRun'.
> org.gradle.api.model.ObjectFactory.fileCollection()Lorg/gradle/api/file/ConfigurableFileCollection;

I searched and found ObjectFactory.fileCollection() is available sicne 5.3

Hence I guess the following code caused the issues and required the gradle upgrade

big-guy · 2019-08-06T17:19:10Z

Hmm, if that's the only problem, we can probably make it work with 5.2.1

aaronlijie · 2019-08-06T17:32:24Z

Hi @big-guy
Thank you for your reply.
According to the user guide : https://gradle.github.io/playframework/

The build has to use Gradle 5.0 or higher

So I think it is better to not couple with any specific version higher than 5.0.

benmccann · 2020-04-23T16:01:38Z

I've since left LinkedIn, but thought I'd provide some feedback anyway since I was previously working on this code:

JavaScriptMinify could be deleted or moved to a separate plugin. There's no built-in JavaScript minification in Play as far as I'm aware, so I'm not sure why this is included. In Play people most often use a native JavaScript toolchain like webpack though you could also add an sbt plugin like sbt-uglify, but I don't think Play does anything here by default. LinkedIn doesn't need it as far as I'm aware
I'm not sure why there's a PlayIdeaPlugin. Why can't we use Gradle's core IDEA plugin with the proper configuration? If the Play IDEA plugin is doing something special it'd be nice to add some code comments explaining
The Twirl compiler could be made a separate plugin. Twirl can be used outside of Play. The Play sbt plugin automatically includes the Twirl sbt plugin, which can be disabled. LinkedIn doesn't need Twirl. It would also make the codebase more modular and easier to maintain
The tests take a really long time to run. I'm not sure if there's anyway to speed this up, but I gave up after 30 min. The continuous build took about an hour and a half to complete. Splitting out the above packages might at least move those tests into a separate project making each have a smaller test suite

pioterj assigned bmuschko Dec 14, 2018

bmuschko mentioned this issue Apr 4, 2019

IDEA plugin should allow users to modify source directories #91

Merged

Get feedback from LinkedIn about the new plugin #77

Get feedback from LinkedIn about the new plugin #77

Comments

pioterj commented Dec 14, 2018

ymwang1984 commented Jan 8, 2019

big-guy commented Jan 8, 2019

ymwang1984 commented Jan 8, 2019

pioterj commented Jan 9, 2019

ymwang1984 commented Jan 9, 2019 • edited by big-guy Loading

pioterj commented Jan 10, 2019

bmuschko commented Jan 10, 2019

ymwang1984 commented Jan 11, 2019

bmuschko commented Jan 11, 2019

pioterj commented Jan 11, 2019

ymwang1984 commented Jan 11, 2019

bmuschko commented Jan 11, 2019

ymwang1984 commented Jan 11, 2019

bmuschko commented Jan 12, 2019

ymwang1984 commented Jan 16, 2019

bmuschko commented Jan 21, 2019

ymwang1984 commented Jan 22, 2019

bmuschko commented Jan 29, 2019

ymwang1984 commented Jan 29, 2019

ymwang1984 commented Jan 31, 2019

bmuschko commented Feb 6, 2019

bmuschko commented Feb 19, 2019

ymwang1984 commented Feb 20, 2019

bmuschko commented Feb 20, 2019

ymwang1984 commented Apr 1, 2019

aaronlijie commented Apr 2, 2019 • edited Loading

bmuschko commented Apr 2, 2019

aaronlijie commented Apr 2, 2019 • edited Loading

bmuschko commented Apr 2, 2019

aaronlijie commented Apr 2, 2019 • edited Loading

aaronlijie commented Apr 3, 2019

bmuschko commented Apr 4, 2019

bmuschko commented Apr 4, 2019

bmuschko commented Apr 10, 2019

mkurz commented Apr 10, 2019

bmuschko commented Apr 10, 2019

aaronlijie commented Apr 10, 2019 • edited Loading

bmuschko commented Apr 15, 2019

aaronlijie commented Apr 17, 2019

bmuschko commented Apr 29, 2019 • edited Loading

bmuschko commented May 5, 2019

aaronlijie commented Jun 11, 2019

aaronlijie commented Jul 19, 2019

ymwang1984 commented Aug 5, 2019

big-guy commented Aug 6, 2019

aaronlijie commented Aug 6, 2019

big-guy commented Aug 6, 2019

aaronlijie commented Aug 6, 2019

benmccann commented Apr 23, 2020 • edited Loading

ymwang1984 commented Jan 9, 2019 •

edited by big-guy

Loading

aaronlijie commented Apr 2, 2019 •

edited

Loading

aaronlijie commented Apr 2, 2019 •

edited

Loading

aaronlijie commented Apr 2, 2019 •

edited

Loading

aaronlijie commented Apr 10, 2019 •

edited

Loading

bmuschko commented Apr 29, 2019 •

edited

Loading

benmccann commented Apr 23, 2020 •

edited

Loading