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

RM-105: Adding Dockerfile #257

Merged
merged 1 commit into from
Nov 20, 2023
Merged

RM-105: Adding Dockerfile #257

merged 1 commit into from
Nov 20, 2023

Conversation

naswierczek
Copy link
Contributor

  • This will build a container and run all non-integration tests for records-mover

@naswierczek naswierczek force-pushed the rm-105-rm-dev-dockerfile branch 5 times, most recently from 68dbd02 to 540e5e8 Compare November 9, 2023 18:55
Copy link
Contributor

@ryantimjohn ryantimjohn left a comment

Choose a reason for hiding this comment

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

@naswierczek this looks great!

You have a quality job failing: https://app.circleci.com/pipelines/github/bluelabsio/records-mover/3639/workflows/8bb8e207-4583-4766-8991-f9828032bcfd/jobs/48579/parallel-runs/0/steps/0-103

Clean up the additional errors on the MD Linter that you additions added and you should be good to commit!

Additionally, it might be worth us figuring out an auto-formatter instead of a linter so that we don't have to worry about silly errors like these delaying deploying code.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@naswierczek naswierczek force-pushed the rm-105-rm-dev-dockerfile branch from 2da75c8 to ceb4329 Compare November 14, 2023 17:04
@naswierczek
Copy link
Contributor Author

@ryantimjohn Please review and remove your request for changes when you get a chance! :)

Dockerfile.dev Outdated Show resolved Hide resolved
@ryantimjohn
Copy link
Contributor

Also, you can always hit the re-request review button:

Screenshot 2023-11-14 at 4 38 49 PM

to remove the changes requested status!

@naswierczek naswierczek force-pushed the rm-105-rm-dev-dockerfile branch from ceb4329 to 9f37451 Compare November 14, 2023 22:28
@naswierczek naswierczek force-pushed the rm-105-rm-dev-dockerfile branch from 9f37451 to cf735c0 Compare November 15, 2023 19:12
Copy link
Contributor

@ryantimjohn ryantimjohn left a comment

Choose a reason for hiding this comment

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


Copy link
Contributor

@ryantimjohn ryantimjohn left a comment

Choose a reason for hiding this comment

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

Ack, sorry, I'm getting an error trying to build this:

 > [7/7] RUN python -m venv venv     && . venv/bin/activate     && pip install --upgrade pip     && pip install --progress-bar=off -r requirements.txt     && pip install --upgrade --progress-bar=off 'pandas>=1.5.3,<2'     && pip install -e '.[unittest,typecheck]' #:
#10 0.321 + python -m venv venv
#10 0.407 Error: [Errno 2] No such file or directory: '/records-mover/venv/bin/python'
------
executor failed running [/bin/bash -exo pipefail -c python -m venv venv     && . venv/bin/activate     && pip install --upgrade pip     && pip install --progress-bar=off -r requirements.txt     && pip install --upgrade --progress-bar=off 'pandas>=1.5.3,<2'     && pip install -e '.[unittest,typecheck]' #]: exit code: 1

@ryantimjohn
Copy link
Contributor

@naswierczek I think I understand what happened, I think you need to add a .dockerignore which ignores your local venv/, I think this worked for you because you don't have a local venv. I did, which messed up the install step.

@ryantimjohn
Copy link
Contributor

This is what my .dockerignore looks like (a lot like a .gitignore)
Screenshot 2023-11-20 at 9 42 10 AM

Now builds fine:
Screenshot 2023-11-20 at 9 42 29 AM

- This will build a container and run all non-integration tests for
  records-mover
- Added docker-compose to make life simpler
- includes .dockerignore to exclude generated artifacts
	 (copy-pasta of .gitgnore)
- Updating .gitignore
@naswierczek naswierczek force-pushed the rm-105-rm-dev-dockerfile branch from cf735c0 to b8ecafb Compare November 20, 2023 15:36
README.md Show resolved Hide resolved
Copy link
Contributor

@ryantimjohn ryantimjohn left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for addressing all concerns so thoughtfully!

@naswierczek naswierczek added this pull request to the merge queue Nov 20, 2023
Merged via the queue into main with commit 48f8fa6 Nov 20, 2023
28 checks passed
@naswierczek naswierczek deleted the rm-105-rm-dev-dockerfile branch January 3, 2024 15:42
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