search

30x / project-management

Tasks not specific to a given project, exploratory stuff and project management
0 stars 0 forks source link

Discuss using different Nodejs versions #166

Closed whitlockjc closed 8 years ago

whitlockjc commented 8 years ago

The first bit of feedback we've gotten from a customer is the desire to use Node v6.x instead of the Node v4.2.x that kiln locks the images down to. We should discuss if/how we want to support this.

mpnally commented 8 years ago

I just looked on my machine and I'm running 6.3.1, so I'm not surprised this came up. I might have expected that there would be a field in package.json to allow you to record the node version(s) that your app is compatible with, but I don't see anything. Is package.json extensible in the sense that you can put extra stuff in there? If it were that would be an easy way for customers to signal what node version they want. (Of course, we could look for a peer file that we invent, but that is less tidy.) We could then just decide which ones we want to honor.

noahdietz commented 8 years ago

In Slack, @whitlockjc mentioned defaulting to LTS version (Node v4.2.x), but allowing configuration of this through an optional parameter, mirrored by shipyardctl. This is the most straight forward approach.

The image build API command does take a lot of information as is, and this would add another piece, so if there is a way to indicate the version in the package.json like @mpnally indicated, that'd be cool too.

noahdietz commented 8 years ago

The package.json engines property looks to be the answer to supporting this in the app manifest.

noahdietz commented 8 years ago

Using the engines property is looking more difficult. The package.json has to either:

And then we would still need to support parsing all of npm's versioning syntax and determine what the proper image to use is.

I implemented the addition of an API parameter for NodeVersion and it was ~10 LoC change...we can then just point them at the mhart/alpine-node page to reference supported Node versions.

I'm ok with doing the work for the engines route, I just wanted to say there was a mentionable difference in the amount of work to implement these two options.

mpnally commented 8 years ago

I'm the guy on the project that knows the least about GO, but as I remember, the one afternoon I did spend with GO I had no trouble parsing JSON as interface{} and then rummaging around in it the same way you would in Node or Python.

Martin

On Fri, Sep 23, 2016 at 4:09 PM, Noah Dietz notifications@github.com wrote:

Using the engines property is looking more difficult. The package.json has to either:

  • be Unmarshalled into a golang struct that we have to define (which kind of seems like overkill considering we want just the one property)
  • or use some golang package for parsing unknown JSON objects (like this one https://github.com/Jeffail/gabs)

And then we would still need to support parsing all of npm's versioning syntax and determine what the proper image to use is.

I implemented the addition of an API parameter for NodeVersion and it was ~10 LoC change...we can then just point them at the mhart/alpine-node page to reference supported Node versions.

I'm ok with doing the work for the engines route, I just wanted to say there was a mentionable difference in the amount of work to implement these two options.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/30x/project-management/issues/166#issuecomment-249322817, or mute the thread https://github.com/notifications/unsubscribe-auth/ACoA6t8pK19aKm99372orb7OZZiErUa9ks5qtFw0gaJpZM4KE-yj .

Martin Nally, Apigee Engineering

whitlockjc commented 8 years ago

Yes, parsing some JSON via interface{} is really simple: https://github.com/30x/enrober/blob/master/pkg/server/server.go#L1088

whitlockjc commented 8 years ago

We discussed this in scrum and the agreed-upon implementation is to have a specific parameter in kiln/shipyardctl that allows you to specify the mhart/alpine-node image tag. (This means the available values will be dictated by mhart/alpine-node and its available Docker image tags) This value will be optional and will default to 4 whenever not available since Node.js v4 is the current LTS.

We will need to document this and probably mention it in the appropriate places in the shipyardctl help. Also, we do not need to keep this list in our application because if the user specifies a version that is not supported, the image will fail to build because the base Docker image will fail to be located.

We decided not to use engines in package.json because of complexity and because of it allowing you to specify Node.js versions that are not available from mhart/alpine-node.

noahdietz commented 8 years ago

solved with #60 in kiln and #33 in shipyardctl

mpnally commented 8 years ago

Another objection to using the engine property is that the meaning is different. The meaning of the engine property is "the range of Node versions that this code will work on". The meaning we want is "the specific Node version I want to use". Since this is a different meaning, it is better to use a different mechanism.

On Tue, Sep 27, 2016 at 10:20 AM, Jeremy Whitlock notifications@github.com wrote:

We discussed this in scrum and the agreed-upon implementation is to have a specific parameter in kiln/shipyardctl that allows you to specify the mhart/alpine-node image tag. (This means the available values will be dictated by mhart/alpine-node and its available Docker image tags https://hub.docker.com/r/mhart/alpine-node/tags/) We will need to document this and probably mention it in the appropriate places in the shipyardctl help. Also, we do not need to keep this list in our application because if the user specifies a version that is not supported, the image will fail to build because the base Docker image will fail to be located.

We decided not to use engines in package.json because of complexity and because of it allowing you to specify Node.js versions that are not available from mhart/alpine-node.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/30x/project-management/issues/166#issuecomment-249934310, or mute the thread https://github.com/notifications/unsubscribe-auth/ACoA6sFBQLXlvhvNvDZaIeaPjYwA7aD4ks5quVBIgaJpZM4KE-yj .

Martin Nally, Apigee Engineering