Experimental Go GSDK #103
Experimental Go GSDK #103
Conversation
There was a problem hiding this comment.
This is a great start @dgkanatsios! Most of my comments are nits or typos, but there are a few we can discuss on teams if you want since I might not have all the context for the choices made.
experimental/go/internalgsdk.go
Outdated
|
|
||
| if hr.NextScheduledMaintenanceUtc != "" { | ||
| nextScheduledMaintenanceUtc, _ := time.Parse(time.RFC3339, hr.NextScheduledMaintenanceUtc) | ||
| if i.cachedScheduledMaintenanceUtc.IsZero() || nextScheduledMaintenanceUtc.After(i.cachedScheduledMaintenanceUtc) { |
There was a problem hiding this comment.
To be 100% certain you are using it as a UTC value, since the string is from a web call, you should use the .UTC() method.
There was a problem hiding this comment.
do you mean on the hr.NextScheduledMaintenanceUtc value?
There was a problem hiding this comment.
nextScheduledMaintenanceUtc, in this case that one specifically.
experimental/go/internalgsdk.go
Outdated
| { | ||
| if i.currentGameState != GameStateTerminating { | ||
| i.currentGameState = GameStateTerminating | ||
| go func() { |
There was a problem hiding this comment.
this signaling of a heartbeat is a duplicate block from line 187. Please consider making this its own method.
dgkanatsios
force-pushed
the
go
branch
2 times, most recently
from
7dc45ad
to
9065bb1
Compare
| if i.configuration == nil { | ||
| i.configuration, err = i.getConfiguration() | ||
| if err != nil { | ||
| log.Fatal(err) |
There was a problem hiding this comment.
Just curious. Will this work as expected given that this is called before the initializeLogger method on line 80?
There was a problem hiding this comment.
so I had to do this since we rely on the configuration values to initialize the logger. I'm pretty sure (TM) this will work and panic the program, it will just not have the configuration values (which we have failed to retrieve anyway)
| postBody, _ := json.Marshal(hrr) | ||
| requestBody := bytes.NewBuffer(postBody) | ||
|
|
||
| resp, err := http.Post(heartbeatEndpoint, "application/json", requestBody) |
There was a problem hiding this comment.
I noticed that in the Java GSDK we have some retry logic wrapped around this call.
Should we do that here as well?There was a problem hiding this comment.
hm, I followed up the C# GSDK during the Go GSDK creation, it doesn't seem to exist there. Let's keep it simple for this first version, we can add it later.
| logWarn(fmt.Sprintf("Error parsing heartbeat response %s", err)) | ||
| } | ||
|
|
||
| i.updateStateFromHeartbeat(&hbr) |
There was a problem hiding this comment.
Looking at the Java GSDK here (
Terminating state before we process the incoming heartbeat. If we are in that state then we immediately send a heartbeat to the Agent saying that we're in the Terminated state and then halt further processing. I haven't read this entire PR yet, but I don't see anything in this GSDK that sends the Terminated heartbeat to the Agent.
There was a problem hiding this comment.
hm, that's interesting - I don't think we send the Terminated ever on the C# GSDK. We'll need to check this and see what's going on
This PR includes the Go GSDK. Highly experimental, not to be used in production. Supported only on this GitHub repo by opening an issue.