Add close method to node client #748

BradfordMedeiros · 2019-04-27T23:37:09Z

I was using the client and realized I couldnt close it as the streams from the grpc were being held open. I just went ahead and created a close method that cleans up the connection info.

Also updated the examples to follow suit since overkill process.exit no longer needed.
Also, awesome project. This is really amazing great stuff guys.

markmandel · 2019-04-27T23:39:01Z

/cc @steven-supersolid you are our local node.js expert 😄 see any issues?

agones-bot · 2019-04-27T23:52:05Z

Build Succeeded 👏

Build Id: 8451e693-52a8-471f-be16-b8e133f56f87

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:0.10.0-d345a24
image: gcr.io/agones-images/agones-sdk:0.10.0-d345a24
image: gcr.io/agones-images/agones-ping:0.10.0-d345a24
Linux C++ SDK (build): agonessdk-0.10.0-d345a24-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-d345a24.zip

A preview of the website (the last 30 builds are retained):

https://d345a24-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/748/head:pr_748 && git checkout pr_748
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-d345a24

agones-bot · 2019-04-27T23:55:15Z

Build Succeeded 👏

Build Id: 3253e948-31c4-4831-9244-7c92df642400

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:0.10.0-60f23e5
image: gcr.io/agones-images/agones-sdk:0.10.0-60f23e5
image: gcr.io/agones-images/agones-ping:0.10.0-60f23e5
Linux C++ SDK (build): agonessdk-0.10.0-60f23e5-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-60f23e5.zip

A preview of the website (the last 30 builds are retained):

https://60f23e5-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/748/head:pr_748 && git checkout pr_748
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-60f23e5

agones-bot · 2019-05-01T21:27:45Z

Build Failed 😱

Build Id: b35ff169-2c3e-461e-996e-a828eadf60b8

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2019-05-01T23:27:49Z

Build Failed 😱

Build Id: 3b4c9075-554e-4809-ac6e-66800436ba0c

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

steven-supersolid

Looks great although I am unable to run the code until next week. Couple of minor changes requested including adding a test as I'm really keen to maintain coverage.

steven-supersolid · 2019-05-02T07:56:38Z

sdks/nodejs/src/agonesSDK.js

+		this.healthStream = undefined;
+		this.emitters = [];
+	}
+	async close(){


Please can you add a test for this method to maintain code coverage? You can check coverage with the npm script cover

steven-supersolid · 2019-05-02T07:59:55Z

sdks/nodejs/src/agonesSDK.js

 			this.healthStream = this.client.health((error) => {
 				// Ignore error as this can't be caught
 			});
 		}
-		let request = new messages.Empty();
-		this.healthStream.write(request);
+		this.healthStream.write(new messages.Empty());


I prefer to keep the request as a local variable as this can aid debugging, is consistent with other calls that pass a request/message and fewer changes if parameters ever have to be added to the message in future.

markmandel · 2019-05-15T19:39:17Z

@BradfordMedeiros gentle bump on this - are you able to respond to the review comments and/or make the requested changes?

…BradfordMedeiros/agones into add-close-method-to-node-client

BradfordMedeiros · 2019-05-16T04:45:33Z

@markmandel @steven-supersolid
Addressed the comments.

…BradfordMedeiros/agones into add-close-method-to-node-client

agones-bot · 2019-05-16T04:55:47Z

Build Succeeded 👏

Build Id: 6ac2c860-bb1e-4797-b717-71a9f7c0ac26

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:0.10.0-987e6d2
image: gcr.io/agones-images/agones-sdk:0.10.0-987e6d2
image: gcr.io/agones-images/agones-ping:0.10.0-987e6d2
Linux C++ SDK (build): agonessdk-0.10.0-987e6d2-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-987e6d2.zip

A preview of the website (the last 30 builds are retained):

https://987e6d2-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/748/head:pr_748 && git checkout pr_748
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-987e6d2

agones-bot · 2019-05-16T04:59:07Z

Build Succeeded 👏

Build Id: 2f5e5182-89a2-4acc-9fb0-9ab97614dcf9

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:0.10.0-7cbb8aa
image: gcr.io/agones-images/agones-sdk:0.10.0-7cbb8aa
image: gcr.io/agones-images/agones-ping:0.10.0-7cbb8aa
Linux C++ SDK (build): agonessdk-0.10.0-7cbb8aa-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-7cbb8aa.zip

A preview of the website (the last 30 builds are retained):

https://7cbb8aa-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/748/head:pr_748 && git checkout pr_748
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-7cbb8aa

agones-bot · 2019-05-16T05:03:24Z

Build Succeeded 👏

Build Id: 14236cae-54ec-4acd-9a11-3b8ff30c3f47

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:0.10.0-a0e285b
image: gcr.io/agones-images/agones-sdk:0.10.0-a0e285b
image: gcr.io/agones-images/agones-ping:0.10.0-a0e285b
Linux C++ SDK (build): agonessdk-0.10.0-a0e285b-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-a0e285b.zip

A preview of the website (the last 30 builds are retained):

https://a0e285b-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/748/head:pr_748 && git checkout pr_748
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-a0e285b

agones-bot · 2019-05-16T05:20:50Z

Build Succeeded 👏

Build Id: 63534d95-fcf9-42bb-af3d-74709c0c3938

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:0.10.0-ddd7550
image: gcr.io/agones-images/agones-sdk:0.10.0-ddd7550
image: gcr.io/agones-images/agones-ping:0.10.0-ddd7550
Linux C++ SDK (build): agonessdk-0.10.0-ddd7550-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-ddd7550.zip

A preview of the website (the last 30 builds are retained):

https://ddd7550-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/748/head:pr_748 && git checkout pr_748
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-ddd7550

markmandel

I assume this is good to go 😄 looks like all comments have been dealt with 👍

agones-bot · 2019-05-16T06:30:04Z

Build Succeeded 👏

Build Id: ed6d91ea-0fbe-4f34-adf9-99cb12068151

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:0.10.0-abad38a
image: gcr.io/agones-images/agones-sdk:0.10.0-abad38a
image: gcr.io/agones-images/agones-ping:0.10.0-abad38a
Linux C++ SDK (build): agonessdk-0.10.0-abad38a-linux-arch_64.tar.gz
SDK Server: agonessdk-server-0.10.0-abad38a.zip

A preview of the website (the last 30 builds are retained):

https://abad38a-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/748/head:pr_748 && git checkout pr_748
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-abad38a

BradfordMedeiros added 4 commits April 27, 2019 16:15

add close method to node sdk client

cf74686

update example node.js to use client.close instead of process.exit

126f70a

-

4440b88

use const where possible

d345a24

Merge branch 'master' into add-close-method-to-node-client

60f23e5

Merge branch 'master' into add-close-method-to-node-client

1c5f2a5

Merge branch 'master' into add-close-method-to-node-client

782fd75

steven-supersolid reviewed May 2, 2019

View reviewed changes

markmandel added the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label May 8, 2019

markmandel removed feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) labels May 16, 2019

BradfordMedeiros added 3 commits May 15, 2019 21:38

-

081ebb1

Merge branch 'add-close-method-to-node-client' of https://github.com/…

987e6d2

…BradfordMedeiros/agones into add-close-method-to-node-client

Merge branch 'master' into add-close-method-to-node-client

7cbb8aa

BradfordMedeiros added 2 commits May 15, 2019 21:47

-

8a544d0

Merge branch 'add-close-method-to-node-client' of https://github.com/…

a0e285b

…BradfordMedeiros/agones into add-close-method-to-node-client

Merge branch 'master' into add-close-method-to-node-client

ddd7550

markmandel approved these changes May 16, 2019

View reviewed changes

Merge branch 'master' into add-close-method-to-node-client

abad38a

markmandel added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/feature New features for Agones labels May 16, 2019

markmandel added this to the 0.11.0 milestone May 16, 2019

markmandel merged commit 14b890c into googleforgames:master May 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add close method to node client #748

Add close method to node client #748

BradfordMedeiros commented Apr 27, 2019 •

edited

Loading

markmandel commented Apr 27, 2019

agones-bot commented Apr 27, 2019

agones-bot commented Apr 27, 2019

agones-bot commented May 1, 2019

agones-bot commented May 1, 2019

steven-supersolid left a comment

steven-supersolid May 2, 2019

steven-supersolid May 2, 2019

markmandel commented May 15, 2019

BradfordMedeiros commented May 16, 2019

agones-bot commented May 16, 2019

agones-bot commented May 16, 2019

agones-bot commented May 16, 2019

agones-bot commented May 16, 2019

markmandel left a comment

agones-bot commented May 16, 2019

Add close method to node client #748

Add close method to node client #748

Conversation

BradfordMedeiros commented Apr 27, 2019 • edited Loading

markmandel commented Apr 27, 2019

agones-bot commented Apr 27, 2019

agones-bot commented Apr 27, 2019

agones-bot commented May 1, 2019

agones-bot commented May 1, 2019

steven-supersolid left a comment

Choose a reason for hiding this comment

steven-supersolid May 2, 2019

Choose a reason for hiding this comment

steven-supersolid May 2, 2019

Choose a reason for hiding this comment

markmandel commented May 15, 2019

BradfordMedeiros commented May 16, 2019

agones-bot commented May 16, 2019

agones-bot commented May 16, 2019

agones-bot commented May 16, 2019

agones-bot commented May 16, 2019

markmandel left a comment

Choose a reason for hiding this comment

agones-bot commented May 16, 2019

BradfordMedeiros commented Apr 27, 2019 •

edited

Loading