Counters and Lists: Remove Bool Returns #3738

Kalaiselvi84 · 2024-03-27T23:25:47Z

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind hotfix
/kind release

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Closes #3737

Special notes for your reviewer:

Kalaiselvi84 · 2024-03-27T23:26:52Z

Once the testing is completed, I will move this PR to ready for review.

agones-bot · 2024-03-27T23:56:38Z

Build Failed 😱

Build Id: 4b2593e5-ac8e-4664-9ed4-ed3d6258b320

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

Kalaiselvi84 · 2024-04-01T22:46:07Z

@markmandel Could you please share the expected output for the methods from which we plan to remove the boolean return type?
Here is the list:
IncrementCounter,
DecrementCounter,
SetCounterCount,
SetCounterCapacity,
SetListCapacity,
AppendListValue,
DeleteListValue.

I've tried testing with IncrementCounter:

nc -u localhost 7654
GET_COUNTER_COUNT rooms
COUNTER: 1
INCREMENT_COUNTER games 1
ERROR: INCREMENT_COUNTER games 1
INCREMENT_COUNTER rooms 1
INCREMENT_COUNTER rooms 1GET_COUNTER_COUNT rooms [Please note that for the success use case, the return value will be `nil`. Should we include (string, error) as return types to include the `\n`?]
COUNTER: 2
INCREMENT_COUNTER rooms 100
ERROR: INCREMENT_COUNTER rooms 100
INCREMENT_COUNTER rooms -1    
ERROR: INCREMENT_COUNTER rooms -1

examples/simple-game-server/main.go

agones-bot · 2024-04-04T21:52:44Z

Build Failed 😱

Build Id: 9647b3b8-5ffc-4eac-a36b-b0796077f4c5

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

sdks/go/alpha.go

agones-bot · 2024-04-06T00:12:21Z

Build Failed 😱

Build Id: 0fffb6b6-e1df-4f05-ad0a-c1e64e4024da

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

markmandel

Just some small tweaks. @igooch did you want to review as well?

sdks/go/alpha.go

agones-bot · 2024-04-10T20:35:06Z

Build Failed 😱

Build Id: 6d49ae21-b80c-4e3f-8559-727b59bc4342

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

igooch

LGTM with a few minor formatting suggestions.

As you probably already know we'll need to push the updated simple-game-server and update the simple-game-server version in framework.go.

sdks/go/alpha.go

…nto cl-rm-bool

agones-bot · 2024-04-16T02:48:06Z

Build Failed 😱

Build Id: 7ea05270-11dd-4b98-8603-474812a9a22d

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

markmandel

LGTM! @igooch any last comments?

Kalaiselvi84 · 2024-04-17T03:04:58Z

The simple-game-server example has been updated with a new image in the registry and the associated files.

agones-bot · 2024-04-17T04:03:07Z

Build Failed 😱

Build Id: ecd3d77b-e22b-4576-a344-8ba0ed3d4636

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

agones-bot · 2024-04-17T16:25:19Z

Build Failed 😱

Build Id: 467082b3-c698-4f20-b29c-722dae9f447e

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

agones-bot · 2024-04-17T18:24:20Z

Build Failed 😱

Build Id: 93da1a87-af2e-4717-a995-6a1980ec341b

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

agones-bot · 2024-04-17T20:48:23Z

Build Failed 😱

Build Id: 776f85a0-c962-437c-818e-8c141541a9c3

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

igooch · 2024-04-18T15:55:11Z

examples/simple-game-server/main.go

@@ -322,26 +323,23 @@ func handleResponse(txt string, s *sdk.SDK, cancel context.CancelFunc) (response
 			responseError = fmt.Errorf("Invalid INCREMENT_COUNTER, should have 2 arguments")
 			return
 		}
-		response, responseError = incrementCounter(s, parts[1], parts[2])
-		addACK = false


When addACK = true then any response will start with "ACK" which why we're seeing actual : "ACK: SUCCESS\n" as the result.

I haven't made any modifications to the value of this addACK.

Here's the code snippet:

if err != nil { return err.Error(), false, err }

That's in the case where there is an error. In the case where there is no error it returns the default addACK = true.

Yes I understand this part, when there is no error, the default value will be addAck = true.
Sorry, I want to make sure I get it right. Could you please share your suggestion differently?

I believe @igooch is saying that when you look at the e2e tests:

fleet_test.go:1745: Error Trace: /go/src/agones.dev/agones/test/e2e/fleet_test.go:1745 Error: Not equal: expected: "SUCCESS: true\n" actual : "ACK: SUCCESS\n"

The reason that the actual response is ACK: SUCCESS, is because you removed the line addACK = false -- whereas it should remain in to remove the "ACK: " from the response we're seeing in e2e tests (and I expect locally as well).

That's a good pickup @igooch - I didn't see the "ACK: SUCCESS" output in #3738 (comment) - whereas the output should just be "SUCCESS".

Thank you for explaining. I've made changes to the code in the main.go and gameserver_test.go files. Please review the testing for the success use case. Once you both confirm the changes, will push a new image to production.

$ nc -u localhost 7654 GET_COUNTER_COUNT rooms COUNTER: 1 INCREMENT_COUNTER rooms 1 SUCCESS INCREMENT_COUNTER rooms 1 SUCCESS DECREMENT_COUNTER rooms 1 SUCCESS SET_COUNTER_COUNT rooms 5 SUCCESS SET_COUNTER_CAPACITY rooms 6 SUCCESS SET_LIST_CAPACITY players 1000 SUCCESS APPEND_LIST_VALUE players test3 SUCCESS DELETE_LIST_VALUE players test2 SUCCESS

That looks good!

@igooch I look forward to hearing your thoughts at your convenience!🙂

agones-bot · 2024-04-19T02:28:10Z

Build Failed 😱

Build Id: eb429176-2c8f-4179-978d-ca3f8c1c2505

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

…nto cl-rm-bool

agones-bot · 2024-04-19T18:30:21Z

Build Failed 😱

Build Id: 63343479-86dd-4cb5-be5d-ce7459104a6b

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

agones-bot · 2024-04-19T21:03:06Z

Build Failed 😱

Build Id: 42cf8722-c134-480c-af86-d331f3b90425

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

zmerlynn · 2024-04-19T21:05:12Z

@Kalaiselvi84

Step #1 - "e2e-feature-gates":         	Error Trace:	/go/src/agones.dev/agones/test/e2e/fleet_test.go:1662
Step #1 - "e2e-feature-gates":         	Error:      	Not equal: 
Step #1 - "e2e-feature-gates":         	            	expected: "SUCCESS: true\n"
Step #1 - "e2e-feature-gates":         	            	actual  : "SUCCESS\n"
Step #1 - "e2e-feature-gates":         	            	
Step #1 - "e2e-feature-gates":         	            	Diff:
Step #1 - "e2e-feature-gates":         	            	--- Expected
Step #1 - "e2e-feature-gates":         	            	+++ Actual
Step #1 - "e2e-feature-gates":         	            	@@ -1,2 +1,2 @@
Step #1 - "e2e-feature-gates":         	            	-SUCCESS: true
Step #1 - "e2e-feature-gates":         	            	+SUCCESS

agones-bot · 2024-04-19T21:35:07Z

Build Failed 😱

Build Id: 5de71090-0210-4f07-8689-0f96a5305e37

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

agones-bot · 2024-04-19T22:37:59Z

Build Succeeded 👏

Build Id: e1c4fba8-7a19-478d-a4e2-66c0c9c0b642

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

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

https://8e6d7b3-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3738/head:pr_3738 && git checkout pr_3738
helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.40.0-dev-8e6d7b3-amd64

Kalaiselvi84 added 2 commits March 27, 2024 22:45

Counters and Lists Methods: Remove Boolean Returns

0ae2d9c

change in main.go and gameserver_test.go

b89c673

github-actions bot added kind/other size/M labels Mar 27, 2024

Kalaiselvi84 changed the title ~~Cl rm bool~~ Counters and Lists: Remove Bool Returns Mar 27, 2024

Kalaiselvi84 requested a review from zmerlynn March 30, 2024 00:05

Kalaiselvi84 requested a review from markmandel April 2, 2024 21:43

markmandel reviewed Apr 3, 2024

View reviewed changes

examples/simple-game-server/main.go Outdated Show resolved Hide resolved

Modify incrementCounter

5b644a8

github-actions bot added the size/S label Apr 4, 2024

Kalaiselvi84 added 2 commits April 4, 2024 21:21

change project name

5c2d4b3

update sdk-client-test.go

542188c

markmandel reviewed Apr 5, 2024

View reviewed changes

sdks/go/alpha.go Show resolved Hide resolved

Kalaiselvi84 added 2 commits April 5, 2024 22:08

C-L methods: remove bool returns

e303e6f

Merge branch 'main' into cl-rm-bool

bf36272

Kalaiselvi84 marked this pull request as ready for review April 5, 2024 22:13

project update

a628dcc

markmandel reviewed Apr 10, 2024

View reviewed changes

sdks/go/alpha.go Outdated Show resolved Hide resolved

sdks/go/alpha.go Outdated Show resolved Hide resolved

sdks/go/alpha.go Outdated Show resolved Hide resolved

error string should not start with uppercase

545a95b

igooch reviewed Apr 11, 2024

View reviewed changes

sdks/go/alpha.go Show resolved Hide resolved

sdks/go/alpha.go Show resolved Hide resolved

Kalaiselvi84 added 3 commits April 16, 2024 00:46

change err to nil

61b137c

Merge branch 'cl-rm-bool' of https://github.com/Kalaiselvi84/agones i…

f10ba34

…nto cl-rm-bool

Merge branch 'main' into cl-rm-bool

af55624

markmandel approved these changes Apr 16, 2024

View reviewed changes

Kalaiselvi84 added 2 commits April 17, 2024 02:56

Bump simple-game-server image

73f5dde

Merge branch 'main' into cl-rm-bool

7081403

Merge branch 'main' into cl-rm-bool

50a357f

igooch reviewed Apr 18, 2024

View reviewed changes

SUCCESS response update: remove ACK and include new line

2062134

markmandel mentioned this pull request Apr 19, 2024

Bump golang.org/x/net from 0.19.0 to 0.23.0 #3793

Merged

zmerlynn and others added 3 commits April 19, 2024 10:20

Merge branch 'main' into cl-rm-bool

0710b86

bump sgs:0.30

4ff3973

Merge branch 'cl-rm-bool' of https://github.com/Kalaiselvi84/agones i…

1049c50

…nto cl-rm-bool

Kalaiselvi84 and others added 2 commits April 19, 2024 18:42

Merge branch 'main' into cl-rm-bool

788f7be

Merge branch 'main' into cl-rm-bool

1f54fd3

zmerlynn enabled auto-merge (squash) April 19, 2024 20:36

remove bool from fleet_test.com

8e6d7b3

zmerlynn merged commit 53a70b3 into googleforgames:main Apr 19, 2024
4 checks passed

Kalaiselvi84 added kind/cleanup Refactoring code, fixing up documentation, etc kind/breaking Breaking change and removed kind/other labels Apr 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Counters and Lists: Remove Bool Returns #3738

Counters and Lists: Remove Bool Returns #3738

Kalaiselvi84 commented Mar 27, 2024

Kalaiselvi84 commented Mar 27, 2024

agones-bot commented Mar 27, 2024

Kalaiselvi84 commented Apr 1, 2024

agones-bot commented Apr 4, 2024

agones-bot commented Apr 6, 2024

markmandel left a comment

agones-bot commented Apr 10, 2024

igooch left a comment

agones-bot commented Apr 16, 2024

markmandel left a comment

Kalaiselvi84 commented Apr 17, 2024

agones-bot commented Apr 17, 2024

agones-bot commented Apr 17, 2024

agones-bot commented Apr 17, 2024

agones-bot commented Apr 17, 2024

igooch Apr 18, 2024

Kalaiselvi84 Apr 18, 2024

igooch Apr 18, 2024

Kalaiselvi84 Apr 18, 2024

markmandel Apr 18, 2024

markmandel Apr 18, 2024

Kalaiselvi84 Apr 18, 2024

markmandel Apr 19, 2024

Kalaiselvi84 Apr 19, 2024

agones-bot commented Apr 19, 2024

agones-bot commented Apr 19, 2024

agones-bot commented Apr 19, 2024

zmerlynn commented Apr 19, 2024

agones-bot commented Apr 19, 2024

agones-bot commented Apr 19, 2024

Counters and Lists: Remove Bool Returns #3738

Counters and Lists: Remove Bool Returns #3738

Conversation

Kalaiselvi84 commented Mar 27, 2024

Kalaiselvi84 commented Mar 27, 2024

agones-bot commented Mar 27, 2024

Kalaiselvi84 commented Apr 1, 2024

agones-bot commented Apr 4, 2024

agones-bot commented Apr 6, 2024

markmandel left a comment

Choose a reason for hiding this comment

agones-bot commented Apr 10, 2024

igooch left a comment

Choose a reason for hiding this comment

agones-bot commented Apr 16, 2024

markmandel left a comment

Choose a reason for hiding this comment

Kalaiselvi84 commented Apr 17, 2024

agones-bot commented Apr 17, 2024

agones-bot commented Apr 17, 2024

agones-bot commented Apr 17, 2024

agones-bot commented Apr 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Apr 19, 2024

agones-bot commented Apr 19, 2024

agones-bot commented Apr 19, 2024

zmerlynn commented Apr 19, 2024

agones-bot commented Apr 19, 2024

agones-bot commented Apr 19, 2024