Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated baas-test-server to use the arm64 docker image #6304

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

kraenhansen
Copy link
Member

What, How & Why?

We still need to update the instructions for how to get the AWS credentials, but with this PR the BaaS test server will be pulled from AWS ECR and will run in an Arm64 compatible docker container 🎉

@kraenhansen kraenhansen self-assigned this Dec 5, 2023
@kraenhansen kraenhansen requested a review from elle-j December 5, 2023 16:03
Copy link

Coverage Status

coverage: 86.51%. remained the same
when pulling f7a9cb3 on kh/docker-baas-test-server
into 0ae91f6 on main.

Copy link
Contributor

@kneth kneth left a comment

Choose a reason for hiding this comment

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

LGTM

const dylibFilename = "libstitch_support.dylib";
const dylibUsrLocalLibPath = "/usr/local/lib/" + dylibFilename;
const assistedAggPath = path.resolve(baasPath, "assisted_agg");
const { AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY } = process.env;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that you don't need to store them in .env?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep - or provide them to the process on the command-line, but the dotenv.config() above helps load them from a file.

Copy link
Contributor

@elle-j elle-j left a comment

Choose a reason for hiding this comment

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

-100 lines 🎉 Nice!

};

function assertImage(value: unknown): asserts value is Image {
assert(typeof value === "object" && value !== null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert(typeof value === "object" && value !== null);
assert(typeof value === "object" && value !== null && !Array.isArray(value));

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's fine. Without the explicit check for array, the property checks below will return undefined and fail their assertions.

};

function assertImagesResponse(value: unknown): asserts value is ImagesResponse {
assert(typeof value === "object" && value !== null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert(typeof value === "object" && value !== null);
assert(typeof value === "object" && value !== null && !Array.isArray(value));

const object = value as Record<string, unknown>;
assert(Array.isArray(object.allBranches));
const { images } = object;
assert(typeof images === "object" && images !== null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert(typeof images === "object" && images !== null);
assert(typeof images === "object" && images !== null && !Array.isArray(images));

@kraenhansen kraenhansen merged commit 123b43d into main Dec 5, 2023
25 of 28 checks passed
@kraenhansen kraenhansen deleted the kh/docker-baas-test-server branch December 5, 2023 17:27
bimusiek pushed a commit to bimusiek/realm-js that referenced this pull request Mar 14, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants