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

node 12 -> node 16, grpc -> grpc-js #630

Merged
merged 9 commits into from
Nov 25, 2021
Merged

node 12 -> node 16, grpc -> grpc-js #630

merged 9 commits into from
Nov 25, 2021

Conversation

askmeegs
Copy link
Contributor

@askmeegs askmeegs commented Nov 22, 2021

I came across these upgrades while trying to fix #538 - our Node and grpc JS versions were out of date and/or deprecated. This turned out to not be the cause of the memory leak but we should probably still upgrade.

changelog

  1. Upgrades the node base images for currency and payment service from Node 12 to Node 16. (Node 12 is no longer the active version of Node and will go out of maintenance in April 2022.
  2. Upgrades the grpc package used in currency and payment service from grpc to grpc-js. grpc is deprecated. See Migrating to grpc-js for the bind-async change you see in this PR.

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 22, 2021
@askmeegs askmeegs marked this pull request as ready for review November 23, 2021 13:34
@askmeegs askmeegs requested a review from a team as a code owner November 23, 2021 13:34
@github-actions
Copy link

🚲 PR staged at http://34.73.14.146

Copy link
Member

@Shabirmean Shabirmean left a comment

Choose a reason for hiding this comment

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

Thank you for the change. Left a comment!

@github-actions
Copy link

🚲 PR staged at http://34.73.14.146

@Shabirmean Shabirmean requested review from a team and bourgeoisor November 25, 2021 15:12
@Shabirmean Shabirmean requested a review from a team November 25, 2021 15:14
@Shabirmean
Copy link
Member

I have updated the PR with some minor improvements. The diff between the last commit from the author and all my commits can be viewed at this link:

96c1a7f...e568cfd

@NimJay NimJay self-requested a review November 25, 2021 18:49
Copy link
Collaborator

@NimJay NimJay left a comment

Choose a reason for hiding this comment

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

I have tried skaffold dev and did a quick manual test of my deployment (on my GKE cluster).
The changes also look good, and yes, Node.js 16 is the way to go today.
Approved!

Thanks, @Shabirmean, for taking over this pull-request and tidying it up. 🙏

src/paymentservice/package.json Show resolved Hide resolved
src/paymentservice/server.js Show resolved Hide resolved
@Shabirmean Shabirmean requested review from Shabirmean and removed request for Shabirmean November 25, 2021 19:36
@Shabirmean Shabirmean merged commit 3f437db into master Nov 25, 2021
@Shabirmean Shabirmean deleted the node-fix-november branch November 25, 2021 19:47
fty4 added a commit to fty4/gcp-microservices-demo that referenced this pull request Dec 14, 2021
fty4 added a commit to fty4/gcp-microservices-demo that referenced this pull request Dec 15, 2021
sitaramkm pushed a commit to sitaramkm/microservices-demo that referenced this pull request Mar 27, 2022
* save work so far

* upgrade paymentservice

* cleanup

* Currency package lock

* payment packagelog

* cleanup: fix indent and minor changes

* chore: test commit

* chore: revert the test commit

* cleanup: update the cloud profiler version

Co-authored-by: shabirmean <7249208+Shabirmean@users.noreply.github.com>
Co-authored-by: shabirmean <shabirmean@google.com>
D-Mwanth pushed a commit to D-Mwanth/microservices-demo that referenced this pull request Mar 6, 2024
* save work so far

* upgrade paymentservice

* cleanup

* Currency package lock

* payment packagelog

* cleanup: fix indent and minor changes

* chore: test commit

* chore: revert the test commit

* cleanup: update the cloud profiler version

Co-authored-by: shabirmean <7249208+Shabirmean@users.noreply.github.com>
Co-authored-by: shabirmean <shabirmean@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible memory leak in NodeJS / Python services
3 participants