Playfab allocator - #2
Conversation
|
|
||
| [CloudCodeFunction(nameof(Allocate))] | ||
| public async Task<AllocateResponse> Allocate(IExecutionContext context, AllocateRequest request) | ||
| { | ||
| try | ||
| { | ||
| PlayFabSettings.staticSettings.DeveloperSecretKey = (await _gameApiClient.SecretManager.GetSecret(context, DeveloperSecretKey)).Value; | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| const string error = $"An error occured when retrieving secrets for key '{DeveloperSecretKey}'."; | ||
| LogError(error, e); | ||
| return new AllocateResponse(AllocateStatus.Error) { Message = AllocationUserFriendlyError }; | ||
| } |
There was a problem hiding this comment.
We can copy/paste the try/catch for all GetSecret calls or we could also introduce a TryGetSecret like so:
| [CloudCodeFunction(nameof(Allocate))] | |
| public async Task<AllocateResponse> Allocate(IExecutionContext context, AllocateRequest request) | |
| { | |
| try | |
| { | |
| PlayFabSettings.staticSettings.DeveloperSecretKey = (await _gameApiClient.SecretManager.GetSecret(context, DeveloperSecretKey)).Value; | |
| } | |
| catch (Exception e) | |
| { | |
| const string error = $"An error occured when retrieving secrets for key '{DeveloperSecretKey}'."; | |
| LogError(error, e); | |
| return new AllocateResponse(AllocateStatus.Error) { Message = AllocationUserFriendlyError }; | |
| } | |
| bool TryGetSecret(IExecutionContext context, string secretKey, out string secretValue) | |
| { | |
| try | |
| { | |
| secretValue = _gameApiClient.SecretManager.GetSecret(context, secretKey).Result.Value; | |
| return true; | |
| } | |
| catch (Exception e) | |
| { | |
| var error = $"An error occured when retrieving secrets for key '{secretKey}'."; | |
| LogError(error, e); | |
| } | |
| secretValue = string.Empty; | |
| return false; | |
| } | |
| [CloudCodeFunction(nameof(Allocate))] | |
| public async Task<AllocateResponse> Allocate(IExecutionContext context, AllocateRequest request) | |
| { | |
| if (!TryGetSecret(context, DeveloperSecretKey, out var developerSecret)) | |
| { | |
| return new AllocateResponse(AllocateStatus.Error) { Message = AllocationUserFriendlyError }; | |
| } | |
| PlayFabSettings.staticSettings.DeveloperSecretKey = developerSecret; |
With the caveat that it's a blocking call, not an async call anymore.
What do you think ?
There was a problem hiding this comment.
We shouldnt be introducing blocking calls. Task.Result is a dangerous code smell.
| const string DeveloperSecretKey = "DEVELOPER_SECRET_KEY"; | ||
|
|
||
| /// <summary> | ||
| /// You will need to set up a secret in the <a |
There was a problem hiding this comment.
Should this be a local const rather than a secret?
There was a problem hiding this comment.
It was but then I checked how GameLift allocator was doing it and it is using the secret store.
So I changed it to be consistent with GameLift allocator.
| /// href="https://cloud.unity.com">Unity Dashboard</a> with | ||
| /// the <c>TITLE_ID</c> key containing your PlayFab Title Id. | ||
| /// </summary> | ||
| const string PlayfabTitleId = "TITLE_ID"; |
There was a problem hiding this comment.
Should this be a local const rather than a secret?
|
|
||
| [CloudCodeFunction(nameof(Allocate))] | ||
| public async Task<AllocateResponse> Allocate(IExecutionContext context, AllocateRequest request) | ||
| { | ||
| try | ||
| { | ||
| PlayFabSettings.staticSettings.DeveloperSecretKey = (await _gameApiClient.SecretManager.GetSecret(context, DeveloperSecretKey)).Value; | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| const string error = $"An error occured when retrieving secrets for key '{DeveloperSecretKey}'."; | ||
| LogError(error, e); | ||
| return new AllocateResponse(AllocateStatus.Error) { Message = AllocationUserFriendlyError }; | ||
| } |
There was a problem hiding this comment.
We shouldnt be introducing blocking calls. Task.Result is a dangerous code smell.
| { | ||
| AllocationData = new Dictionary<string, object> | ||
| { | ||
| { "sessionId", allocationResult.Result.SessionId }, |
There was a problem hiding this comment.
Only "sessionId" is used in Poll. Why set the other values?
There was a problem hiding this comment.
That's true.
Same reason as above, I aligned on what GameLift allocator was doing to be consistent.
28f53c4 to
229de12
Compare
No description provided.