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

GODRIVER-2989 [master] A Dockerfile for local development #1428

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

blink1073
Copy link
Member

Cherry-pick of #1381 from v1 to master

Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Co-authored-by: Qingyang Hu <103950869+qingyang-hu@users.noreply.github.com>
Co-authored-by: Preston Vasquez <prestonvasquez@icloud.com>
Co-authored-by: Matt Dale <9760375+matthewdale@users.noreply.github.com>
Co-authored-by: Eng Zer Jun <engzerjun@gmail.com>
(cherry picked from commit 6d7a981)
@blink1073 blink1073 requested a review from a team as a code owner October 16, 2023 20:38
@blink1073 blink1073 requested review from prestonvasquez and removed request for a team October 16, 2023 20:38
Copy link
Collaborator

@prestonvasquez prestonvasquez left a comment

Choose a reason for hiding this comment

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

I didn't catch the v1 PR, I just have two minor suggestions.

@@ -111,6 +114,24 @@ The usage of host.docker.internal comes from the [Docker networking documentatio

There is currently no arm64 support for the go1.x runtime, see [here](https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html). Known issues running on linux/arm64 include the inability to network with the localhost from the public.ecr.aws/lambda/go Docker image.

### Testing in Docker

We support local testing in Docker. Ensure ``docker`` is installed and running, and then run:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we include something about setting the DRIVER_TOOLS environment variable, rather than being informed only via error? E.g.

To test using docker, you will need to set the DRIVERS_TOOLS environment variable to point to a local clone of the drivers-evergreen-tools repository. This is essential for running the testing matrix in a container. You can set the DRIVERS_TOOLS variable in your shell profile or in your project-specific environment.

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 applied this comment in the follow up PR: #1430

We support local testing in Docker. Ensure ``docker`` is installed and running, and then run:

```bash
bash etc/run_docker.sh
Copy link
Collaborator

@prestonvasquez prestonvasquez Oct 18, 2023

Choose a reason for hiding this comment

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

It could be helpful to make this a make target:

.PHONY: build-tests-docker
build-tests-docker:
	etc/run_docker.sh

Copy link
Member Author

Choose a reason for hiding this comment

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

It gets tricky with how to pass args. I thought we eventually want to remove the Makefile altogether?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Gotcha, that makes sense. Thanks!

@blink1073 blink1073 merged commit 86d6fb7 into mongodb:master Oct 19, 2023
30 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants