Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

CN environment support #1260

Merged
merged 9 commits into from
Jan 29, 2020
Merged

CN environment support #1260

merged 9 commits into from
Jan 29, 2020

Conversation

zeroZshadow
Copy link
Contributor

@zeroZshadow zeroZshadow commented Jan 29, 2020

Description

Add support for the cn-production environment for workers and deployment launcher.
Upgraded the Worker and Platform SDK to 14.4.0, as the locator cannot connect otherwise.
Removed use of deprecated network protocols (Tests still exist)

Tests

Fully tested deployment launcher window to work.
Connected a client to a cloud deployment in cn-production using the locator.

Documentation

  • Changelog

@improbable-prow-robot improbable-prow-robot added jira/no-ticket Indicates a PR has no corresponding JIRA ticket do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 300-599 lines, ignoring generated files. A: build-system Area: Build system feature module A: core Area: Core GDK A: mobile Area: Mobile integration A: tooling Area: Tooling labels Jan 29, 2020
@zeroZshadow zeroZshadow force-pushed the feature/environment-support branch from 3596f6c to 0e86df7 Compare January 29, 2020 15:04
@zeroZshadow
Copy link
Contributor Author

Adding support for the cn-production environment to our init.sh script will be a separate PR.

{
// These are the last tested values
Heartbeat = new HeartbeatParameters
DownstreamHeartbeat = new HeartbeatParameters
Copy link
Contributor

Choose a reason for hiding this comment

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

what are the defaults?

Copy link
Contributor

Choose a reason for hiding this comment

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

They were the last tested values :trollface:

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
.WithArgs("file", "zip", $"--output=\"{Path.GetFullPath(zipPath)}\"",
RedirectedProcess
.Spatial("file", "zip")
.WithArgs($"--output=\"{Path.GetFullPath(zipPath)}\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

So looking at this API, what happens if I put the current args with file and zip?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you give an example?

Copy link
Contributor

Choose a reason for hiding this comment

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

RedirectedProcess.Spatial("file", "zip", "--output=./something.zip", ...)

};

var tokenFile = possibleTokenFiles.FirstOrDefault(File.Exists);
if (!string.IsNullOrEmpty(tokenFile))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this check required if we've ran it through File.Exists on the line above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is also copied from the Platform SDK

}

// Fail if no form of credentials could be found.
throw new NoRefreshTokenFoundException("");
Copy link
Contributor

Choose a reason for hiding this comment

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

Put some message in this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has a message internally which states it was unable to find the token file locally.

@@ -181,6 +178,11 @@ private static string ModifySimulatedPlayerLaunchJson(Options.CreateSimulated op
var content = new ByteArrayContent(bytes);
content.Headers.Add("Content-MD5", snapshotToUpload.Checksum);

if (options.Environment == "cn-production")
{
content.Headers.Add("x-amz-server-side-encryption", "AES256");
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @MatthewSandfordImprobable for the code

{
// These are the last tested values
Heartbeat = new HeartbeatParameters
DownstreamHeartbeat = new HeartbeatParameters
Copy link
Contributor

Choose a reason for hiding this comment

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

They were the last tested values :trollface:

@zeroZshadow zeroZshadow marked this pull request as ready for review January 29, 2020 17:43
@improbable-prow-robot improbable-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 29, 2020
@zeroZshadow zeroZshadow merged commit 8d29b53 into develop Jan 29, 2020
@zeroZshadow zeroZshadow deleted the feature/environment-support branch January 29, 2020 18:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A: build-system Area: Build system feature module A: core Area: Core GDK A: mobile Area: Mobile integration A: tooling Area: Tooling jira/no-ticket Indicates a PR has no corresponding JIRA ticket size/XL Denotes a PR that changes 300-599 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants