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

Containerization #22

Merged
merged 2 commits into from
Dec 5, 2022
Merged

Containerization #22

merged 2 commits into from
Dec 5, 2022

Conversation

kaitj
Copy link
Collaborator

@kaitj kaitj commented Nov 30, 2022

This PR adds and updates files required to containerize labelmerge. Also manually deploying this to my docker hub, but we can also add in the necessary github action to deploy to the khanlab org docker hub

  • Removed the transforms that don't seem necesssary at this point
  • Added .dockerignore to ignore .git (could potentially also include .github)
  • Add dockerfile to build a working docker image with the correct entrypoint
    • Used python:3.9-slim-bullseye as base container due to bug with installed verison of python in debian:bullseye (site-packages weren't in the sys.path up to around python3.9.7; this was later patched and this base container contains those fixes)
    • Updated run.py to use /usr/bin/env python3 rather than /usr/bin/python3 which couldn't find python from the base container

Also fixes a bug with the base vs overlay config (this might have been introduced when I rebased from main). The shallow copy (dict.copy()) was causing the base_desc to be replaced by the overlay_desc. To fix this, deepcopy is used.

Proposed changes

Describe the changes implemented in this pull request. If the changes fix a bug or resolves a feature request, be sure to link the issue. Please explain the issue and how the changes address the issue.

Types of changes

What types of changes does your code introduce? Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionalitiy)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!

  • Changes have been tested to ensure that fix is effective or that a feature works.
  • Changes pass the unit tests
  • Code has been run through the poe quality task
  • I have included necessary documentation or comments (as necessary)
  • Any dependent changes have been merged and published

Notes

All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.

@kaitj kaitj added the enhancement New feature or request label Nov 30, 2022
@kaitj kaitj added this to the v0.1.x milestone Nov 30, 2022
- Removed the transforms that don't seem necesssary at this point
- Added .dockerignore to ignore .git (could potentially also include
.github)
- Add dockerfile to build a working docker image with the correct
entrypoint
  - Used python:3.9-slim-bullseye as base container due to bug with
installed verison of python in debian:bullseye (site-packages weren't in
the sys.path up to around python3.9.7; this was later patched and this
base container contains those fixes)
  - Updated run.py to use "/usr/bin/env python3" rather than
"usr/bin/python3" which couldn't find python from the base container
@kaitj kaitj removed this from the v0.1.x milestone Nov 30, 2022
Config was overwriting the base desc with the overlay desc when only
using .copy(). This was resolved by deepcopying separate configs for
base and overlay.
@tkkuehn tkkuehn merged commit c7cc6c6 into main Dec 5, 2022
@tkkuehn tkkuehn deleted the docker branch December 5, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants