Skip to content
This repository has been archived by the owner on Mar 1, 2022. It is now read-only.

node: compile protos into JSON #678

Merged
merged 4 commits into from
Jul 31, 2019
Merged

Conversation

alexander-fenster
Copy link
Contributor

This PR adds some logic for artman post-processing that compiles all the proto files into JSON for nodejs_gapic tasks. The resulting JSON file (that will appear in all client libraries) is not yet used but very soon will be!

Also, upgrading Node.js inside artman Docker image to v10 (it's now v4 but it was not used anywhere before this PR :) ), and installing google-gax globally (to have the compileProtos script).

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 31, 2019
ENV PATH /opt/node-v10.16.0-linux-x64/bin:$PATH

# Install google-gax for Node.js
RUN npm install -g google-gax@^1.2.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh bother. What's going to keep that version up to date?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... semver ^1.2.1 will allow everything up to 2.0, and it actually prevents me from possible breaking changes in google-gax. It's pretty much the same way as we do in package.json, isn't it?

&& rm -rf /var/lib/apt/lists/*

# Download and unpack Node.js v10
RUN mkdir -p /opt \
&& curl -L https://nodejs.org/dist/v10.16.0/node-v10.16.0-linux-x64.tar.xz -o /opt/node-v10.16.0-linux-x64.tar.xz \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Little worried about how we keep the version of nodejs you're running here up to date.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, first of all, it does not make it any worse because previously it was Node 4 from Ubuntu :) Then, this version of Node.js is only used for compiling protos. I suggest we leave both versions (Node.js and google-gax) hardcoded here for now (same way as we do for other stuff in this Dockerfile such as protoc version, etc.) and update as needed.

Actually, we'll get rid of artman soon (I hope so) so who cares?

@alexander-fenster alexander-fenster merged commit 4cc5471 into master Jul 31, 2019
@noahdietz noahdietz mentioned this pull request Aug 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants