Pending PRs

[Ben McCann]

Hey guys,

Would someone be able to merge the pending PRs at https://github.com/typesafehub/js-engine/pulls?  Neither Matthias nor myself have merge access to that repo.

Thanks,

Ben

[Christopher Hunt]

It looks as though CI is failing there for all three of those PRs… ideas?

--
You received this message because you are subscribed to the Google Groups "Play framework dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Christopher Hunt
Technical Lead, Lightbend Production Suite

@huntchr

UTC+11

[Ben McCann]

It seems to be a flaky test. I've had it both pass and fail for me locally. It seems it's been that way since it was originally committed as I went back in the git history to the original commit and failed for me at that point in time as well

Perhaps James knows more about this as it looks like he wrote the test originally?

[error]   x not leak threads
[error]    '1' is not equal to '0' (TriremeSpec.scala:77)

Thanks,

Ben

On Thu, Jan 12, 2017 at 2:54 PM, Christopher Hunt <christop...@lightbend.com> wrote:

It looks as though CI is failing there for all three of those PRs… ideas?

On 13 Jan 2017, at 8:52 am, Ben McCann <b...@benmccann.com> wrote:

Hey guys,

Would someone be able to merge the pending PRs at https://github.com/typesafehub/js-engine/pulls?  Neither Matthias nor myself have merge access to that repo.

Thanks,

Ben

--
You received this message because you are subscribed to the Google Groups "Play framework dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsub...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

Christopher Hunt
Technical Lead, Lightbend Production Suite

@huntchr

UTC+11

--
You received this message because you are subscribed to the Google Groups "Play framework dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsub...@googlegroups.com.

[James Roper]

Trireme was not straight forward to shut down, there were a number of different thread pools and resource descriptors that needed to be closed in order for it to be properly disposed and garbage collected.  So I wrote a crude test that attempted to detect thread leaks.  After shutting Trireme down, it looks at the threads and sees if there are any that still have Trireme in the name.  If there are, it fails.  So, if this test is failing, then chances are that it has detected a legitimate regression that needs to be fixed, and we need to find out what is not being shut down cleanly.  Possibly we should improve the assertion such that when it fails it emits the name of the thread that is still running, so we can add code to shut it down.

On 13 January 2017 at 11:05, Ben McCann <b...@benmccann.com> wrote:

It seems to be a flaky test. I've had it both pass and fail for me locally. It seems it's been that way since it was originally committed as I went back in the git history to the original commit and failed for me at that point in time as well

Perhaps James knows more about this as it looks like he wrote the test originally?

[error]   x not leak threads
[error]    '1' is not equal to '0' (TriremeSpec.scala:77)

Thanks,

Ben

On Thu, Jan 12, 2017 at 2:54 PM, Christopher Hunt <christopher.hunt@lightbend.com> wrote:

It looks as though CI is failing there for all three of those PRs… ideas?

On 13 Jan 2017, at 8:52 am, Ben McCann <b...@benmccann.com> wrote:

Hey guys,

Would someone be able to merge the pending PRs at https://github.com/typesafehub/js-engine/pulls?  Neither Matthias nor myself have merge access to that repo.

Thanks,

Ben

--
You received this message because you are subscribed to the Google Groups "Play framework dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

Christopher Hunt
Technical Lead, Lightbend Production Suite

@huntchr

UTC+11

--
You received this message because you are subscribed to the Google Groups "Play framework dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Play framework dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsub...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

James Roper
Software Engineer

Lightbend – Build reactive apps!
Twitter: @jroper

[Ben McCann]

I sent a PR to improve the assertion as suggested. I'm not sure I have the background to deal with the root cause bug, but it'd be great if someone could take a look. The other PRs could probably be merged in the meantime since they are failing only due an already present issue and don't introduce any new regressions

Thanks,

Ben

--

James Roper
Software Engineer

Lightbend – Build reactive apps!
Twitter: @jroper

--

[Ben McCann]

Thanks for the merge today, James! The CI is passing for my other RBs now if someone could take a look: https://github.com/typesafehub/js-engine/pulls

Thanks!

-Ben

[Ben McCann]

Hi,

Just a reminder to please take a look at the pending PRs: https://github.com/typesafehub/js-engine/pulls

Thanks,

Ben

[Christopher Hunt]

Done. Thanks

[Ben McCann]

Thank you!  Would it also be possible to get a new release with the fixes?

Thanks,

Ben

To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsub...@googlegroups.com.

[Christopher Hunt]

Sure. I'll give it a go when I get into the office. 

[Christopher Hunt]

Published as 1.2.0.

[Ben McCann]

Awesome! Thank you so much!

[Ben McCann]

To follow up on this, could I also get the pending PRs for the npm repo merged and then a new release cut?

https://github.com/typesafehub/npm/pulls

Thanks!

-Ben

On Tue, Feb 14, 2017 at 10:29 AM, Ben McCann <b...@benmccann.com> wrote:

Awesome! Thank you so much!

[Will Sargent]

Hi Ben,

I've merged the latest PR https://github.com/typesafehub/npm/pull/10 

Given that https://github.com/typesafehub/npm/pull/5 is old and https://github.com/typesafehub/npm/pull/7 is out of date, I don't see any other valid pending PRs.  

When do you want the release cut?

--

Will Sargent

Engineer, Lightbend, Inc.

will.s...@lightbend.com

To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsub...@googlegroups.com.

[Will Sargent]

2.0.0 make sense as a version number?

--

Will Sargent

Engineer, Lightbend, Inc.

will.s...@lightbend.com

[Ben McCann]

Great, thanks! Cutting a release whenever you can would be great.

https://github.com/typesafehub/npm/pull/5 - still seemed valid (even if old), but only affects tests, so doesn't matter if it's before or after a release

https://github.com/typesafehub/npm/pull/2 - I wanted to draw attention to this one because there's one line there that James said he added to fix the release process. I thought you might need it before cutting a release

https://github.com/typesafehub/npm/pull/7 - yeah, this one can be closed now

On Wed, Feb 22, 2017 at 5:59 PM, Will Sargent <will.s...@lightbend.com> wrote:

[Will Sargent]

ReleaseKeys.releaseProcess += sbtrelease.releaseTask(SonatypeKeys.sonatypeReleaseAll in root)

but I don't know the significance there.  I'll try a release using 2.0.0 and let you know how it goes.

--

Will Sargent

Engineer, Lightbend, Inc.

will.s...@lightbend.com

[Christopher Hunt]

On 23 Feb 2017, at 2:00 pm, Will Sargent <will.s...@lightbend.com> wrote:

ReleaseKeys.releaseProcess += sbtrelease.releaseTask(SonatypeKeys.sonatypeReleaseAll in root)

but I don't know the significance there.  I'll try a release using 2.0.0 and let you know how it goes.

It means that it will automatically transition from staging to Maven central without requiring a manual review.

[Will Sargent]

Tests seem to be failing now:

https://travis-ci.org/typesafehub/npm/builds/204655863

I'm having trouble replicating this on my own environment because it requires Node to be installed and I don't quite get the node ecosystem:

=== STDERR ===

npm info it worked if it ends with ok

npm WARN npm npm does not support Node.js v0.10.32

npm WARN npm You should probably upgrade to a newer version of node as we

npm WARN npm can't make any promises that npm will work with this version.

npm WARN npm You can find the latest version at https://nodejs.org/

npm verb cli [ './node',

npm verb cli   '/Users/wsargent/work/npm/target/webjars/npm/lib/npm.js',

npm verb cli   'update',

npm verb cli   '--verbose' ]

npm info using npm@undefined

npm info using no...@v0.10.32

npm verb request uri https://registry.npmjs.org/amdefine

npm verb request no auth needed

npm info attempt registry request try #1 at 9:00:55 AM PST

npm verb request id e156257a2987cdcf

npm verb etag W/"58197303-489f"

npm verb lastModified Wed, 02 Nov 2016 05:00:51 GMT

npm http request GET https://registry.npmjs.org/amdefine

npm verb request uri https://registry.npmjs.org/jsdom

npm verb request no auth needed

npm info attempt registry request try #1 at 9:00:55 AM PST

npm verb etag W/"58ac016a-93fca"

npm verb lastModified Tue, 21 Feb 2017 08:59:22 GMT

npm http request GET https://registry.npmjs.org/jsdom

npm http 304 https://registry.npmjs.org/jsdom

npm verb headers { date: 'Thu, 23 Feb 2017 17:00:56 GMT',

npm verb headers   via: '1.1 varnish',

npm verb headers   'cache-control': 'max-age=300',

npm verb headers   etag: 'W/"58ac016a-93fca"',

npm verb headers   age: '107',

npm verb headers   connection: 'keep-alive',

npm verb headers   'x-served-by': 'cache-sjc3149-SJC',

npm verb headers   'x-cache': 'HIT',

npm verb headers   'x-cache-hits': '1',

npm verb headers   'x-timer': 'S1487869256.215516,VS0,VE0',

npm verb headers   vary: 'Accept-Encoding' }

npm verb etag https://registry.npmjs.org/jsdom from cache

npm verb get saving jsdom to /Users/wsargent/.npm/registry.npmjs.org/jsdom/.cache.json

npm verb correctMkdir /Users/wsargent/.npm correctMkdir not in flight; initializing

npm http 304 https://registry.npmjs.org/amdefine

npm verb headers { date: 'Thu, 23 Feb 2017 17:00:56 GMT',

npm verb headers   via: '1.1 varnish',

npm verb headers   'cache-control': 'max-age=300',

npm verb headers   etag: 'W/"58197303-489f"',

npm verb headers   age: '207',

npm verb headers   connection: 'keep-alive',

npm verb headers   'x-served-by': 'cache-sjc3134-SJC',

npm verb headers   'x-cache': 'HIT',

npm verb headers   'x-cache-hits': '3',

npm verb headers   'x-timer': 'S1487869256.239508,VS0,VE0',

npm verb headers   vary: 'Accept-Encoding' }

npm verb etag https://registry.npmjs.org/amdefine from cache

npm verb get saving amdefine to /Users/wsargent/.npm/registry.npmjs.org/amdefine/.cache.json

npm verb correctMkdir /Users/wsargent/.npm correctMkdir already in flight; waiting

npm verb outdated not updating amdefine because it's currently at the maximum version that matches its specified semver range

npm verb outdated not updating jsdom because it's currently at the maximum version that matches its specified semver range

npm verb exit [ 0, true ]

npm info ok

=== STDOUT ===

[info] NpmSpec

[info]

[info] Npm should

[error]   x perform an update and retrieve resources

[error]     doesn't contain '> node-gyp rebuild' (NpmSpec.scala:47)

[info]

[info]

[info]

[info] Total for specification NpmSpec

[info] Finished in 18 seconds, 557 ms

[info] 1 example, 1 failure, 0 error

[info]

[error] Failed: Total 1, Failed 1, Errors 0, Passed 0

[error] Failed tests:

[error] com.typesafe.jse.NpmSpec

[error] (root/test:test) sbt.TestsFailedException: Tests unsuccessful

[error] Total time: 21 s, completed Feb 23, 2017 9:01:02 AM

>

--

Will Sargent

Engineer, Lightbend, Inc.

will.s...@lightbend.com

[Dominik Dorn]

just go to nodejs.org , download and install the newest version. it will come with npm bundled (had to do this too a few days ago to get node working with angularjs 2/angular)

On Wed, Feb 22, 2017 at 7:12 PM, Christopher Hunt <christopher.hunt@lightbend.com> wrote:

On 23 Feb 2017, at 2:00 pm, Will Sargent <will.s...@lightbend.com> wrote:

ReleaseKeys.releaseProcess += sbtrelease.releaseTask(SonatypeKeys.sonatypeReleaseAll in root)

but I don't know the significance there.  I'll try a release using 2.0.0 and let you know how it goes.

It means that it will automatically transition from staging to Maven central without requiring a manual review.

Christopher Hunt
Technical Lead, Lightbend Production Suite

@huntchr

UTC+11

--
You received this message because you are subscribed to the Google Groups "Play framework dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsub...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Dominik Dorn
http://dominikdorn.com | http://twitter.com/domdorn
XING: https://www.xing.com/profile/Dominik_Dorn
LINKEDIN: http://at.linkedin.com/pub/dominik-dorn/66/b42/bb1/

[Ben McCann]

I'm not sure how this test was passing. I have npm installed. If I do "git reset --hard bfbde9270fc26212ef2f11c45d5f6b569904d6ae" and "sbt test" then I get the same error. Maybe this depends on having some certain version of npm installed. I've tried just about everything else I can think of like "rm -rf ~/.npm", etc.

Perhaps James can shares more details here. I don't have a clear understanding of why we expected "node-gyp rebuild" to show up in the stdout

If we can't figure out what the test was supposed to be testing then I'm okay with reverting it to get the build passing again

-Ben

On Wed, Feb 22, 2017 at 7:12 PM, Christopher Hunt <christop...@lightbend.com> wrote:

On 23 Feb 2017, at 2:00 pm, Will Sargent <will.s...@lightbend.com> wrote:

ReleaseKeys.releaseProcess += sbtrelease.releaseTask(SonatypeKeys.sonatypeReleaseAll in root)

but I don't know the significance there.  I'll try a release using 2.0.0 and let you know how it goes.

It means that it will automatically transition from staging to Maven central without requiring a manual review.

Christopher Hunt
Technical Lead, Lightbend Production Suite

@huntchr

UTC+11

--
You received this message because you are subscribed to the Google Groups "Play framework dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to play-framework-dev+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Dominik Dorn
http://dominikdorn.com | http://twitter.com/domdorn
XING: https://www.xing.com/profile/Dominik_Dorn
LINKEDIN: http://at.linkedin.com/pub/dominik-dorn/66/b42/bb1/

[James Roper]

A few things:

1. The reason for the above failure appears to be that you had already run the test prior, and the last npm upgrade introduced an optimization where node-gyp isn't rerun when it's already been run in a previous build. The solution is to delete the node_modules directory before every test to start with a clean slate.  PR for that here https://github.com/typesafehub/npm/pull/12.

2. This test is not using node.js, so upgrading node.js won't help anything.  It's using Trireme, a JVM implementation of node.js.  Trireme only implements node 0.10.32, and is not likely to provide support for node 4 in the near future, see https://github.com/apigee/trireme#what-version-of-nodejs-does-trireme-support.

3. On my machine, the test fails with a generic timeout, I'm not sure why.

4. On Travis, the test fails with some error related to file permissions. This is possibly a bug in Trireme, or perhaps a result of running npm on node 0.10.32. While it is the test that I wrote that shows this failure, the failure is almost certainly caused by upgrading npm, and so reverting the test will do nothing to solve the problem.

5. If you switch the test to use node instead of trireme, it works (with my change to delete node_modules).

Given all of the above, I suggest to fix we:

1. Switch the test to use Node JS.

2. Implement something that detects whether it's running on Node or Trireme, and if running on Trireme, output a big fat warning saying running npm on Trireme is not supported, with instructions on how to install node and ensure that sbt-js-engine will detect and use that.

Regards,

James

[Ben McCann]

Thanks so much for the help debugging this. I was able to get the test passing by downgrading the version of npm. I agree with you that not supporting npm on Trireme is probably the better long-term solution

James Roper
Software Engineer

Lightbend – Build reactive apps!
Twitter: @jroper

--

[Ben McCann]

I sent a PR to enable detection of nodejs so that we can use the latest version of npm

https://github.com/typesafehub/js-engine/pull/54