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

Dockerfile updates #1937

Merged
merged 7 commits into from
Nov 11, 2020
Merged

Dockerfile updates #1937

merged 7 commits into from
Nov 11, 2020

Conversation

DavisRayM
Copy link
Contributor

Changes / Features implemented

  • Add docker-image-build.yml github workflow
  • Update docker file; Add an intermediate container to install optional dependencies

Steps taken to verify this change does what is intended

  • Built an image

@DavisRayM DavisRayM force-pushed the docker-build-changes branch 2 times, most recently from 4f4fe83 to d7bf935 Compare October 22, 2020 15:31
Copy link
Member

pld commented Oct 22, 2020

Codacy Here is an overview of what got changed by this pull request:

Issues
======
- Added 2
           

See the complete overview on Codacy

RUN ssh-keyscan github.com > /root/.ssh/known_hosts

# Install optional requirements
RUN if [ ! -z "$optional_packages" ]; then pip install ${optional_packages} ; fi
Copy link
Member

Choose a reason for hiding this comment

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

RUN ssh-keyscan github.com > /root/.ssh/known_hosts

# Install optional requirements
RUN if [ ! -z "$optional_packages" ]; then pip install ${optional_packages} ; fi
Copy link
Member

Choose a reason for hiding this comment

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

Codacy found an issue: Use -n instead of ! -z.

@DavisRayM DavisRayM force-pushed the docker-build-changes branch 2 times, most recently from c1edd52 to 8a9cabf Compare November 4, 2020 09:42
ivermac
ivermac previously approved these changes Nov 4, 2020
Copy link
Contributor

@ivermac ivermac left a comment

Choose a reason for hiding this comment

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

Looks good to me but i'd say @ukanga should check it out as well

build:
context: .
dockerfile: Dockerfile
image: onadata:latest
volumes:
# For local development
- ../../.:/srv/onadata
image: onaio/onadata:latest
Copy link
Member

Choose a reason for hiding this comment

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

The point about doing local builds was for development purposes. In my last years of writing code on this repositories this has been the primary way my dev environment worked, from within the container. Perhaps add a docker-compose.prod.yml file instead?


jobs:
main:
runs-on: ubuntu-18.04
Copy link
Member

Choose a reason for hiding this comment

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

Can we start using 20.04 yet?

Copy link
Contributor Author

@DavisRayM DavisRayM Nov 4, 2020

Choose a reason for hiding this comment

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

I believe we can. I'll modify the Dockerfile to use 20.04 as the base image and test it out

ukanga
ukanga previously approved these changes Nov 6, 2020
@DavisRayM DavisRayM merged commit 3390a99 into master Nov 11, 2020
@DavisRayM DavisRayM deleted the docker-build-changes branch November 11, 2020 07:44
@DavisRayM DavisRayM mentioned this pull request Nov 11, 2020
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.

4 participants