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

CI: Implement a publish action to push Docker images #43

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 57 additions & 12 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,24 +1,69 @@
# This workflow will install Conda environment, run tests and lint with a single version of Python.

name: Continuous Integration

on:
schedule:
# Run this workflow at 00:00 on every Monday
- cron: "0 0 * * MON"
push:
branches: [ main, master ]
branches: [ master ]
pull_request:
branches: [ main, master ]
branches: [ master ]
workflow_dispatch:

jobs:
ci:
name: CI
build-test-taswira:
name: Build and Test Taswira
runs-on: ubuntu-latest
env:
DOCKER_BUILDKIT: "1"

steps:
- name: Checkout code
uses: actions/checkout@v2
- name: Lint using pylint
Copy link
Collaborator

Choose a reason for hiding this comment

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

This step that lints the code using pylint is missing in the new workflow.

I had added that step to ensure code quality. It'd be great if we can add it to the new workflow as well.

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 will add that!

run: docker build . --target lint
- name: Test using pytest
run: docker build . --target unit-test

- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: 3.7

- name: Setup Conda
uses: conda-incubator/setup-miniconda@v2
with:
python-version: 3.7
mamba-version: "*"
channels: conda-forge
channel-priority: true
activate-environment: taswira

- name: Cache conda
uses: actions/cache@v2
env:
# Increase this value to reset cache if environment.yml has not changed
CACHE_NUMBER: 0
with:
# Cache the path where dependencies are installed with a key unique to the existing environment.yml
path: /usr/local/miniconda/envs/taswira
key:
${{ runner.os }}-conda-${{ env.CACHE_NUMBER }}-${{ hashFiles('environment.yml') }}
id: envcache

- name: Update Conda Environment
# Setup (or update) the environment for faster installs
run: mamba env update -n taswira -f environment.yml

- name: Install Python package
shell: bash -l {0}
run: |
conda activate taswira
python -m pip install -e .

- name: Run Black
run: |
python -m pip install black
black src --diff
black --check src

- name: Test
shell: bash -l {0}
# Test the package
run: |
conda activate taswira
python -m pytest
58 changes: 58 additions & 0 deletions .github/workflows/docker.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
name: Docker CI/CD

on:
schedule:
Copy link
Collaborator

@abhineet97 abhineet97 Mar 11, 2022

Choose a reason for hiding this comment

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

I don't see the need of publishing a new image every day. Can you elaborate on your intention behind this?

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 was something that we have done with upstream images to ensure that we get to know if something breaks (like on FLINT and FLINT.Example). Hence we add this additional step to build and push the images on a cron schedule. Let me know if we can remove this in case of Taswira.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it. While that sounds like a good step, I don't think it's required for taswira because there's nothing in it that could break overnight. I think, things will only break after a change. Do you agree?

# Run this workflow at 00:00 on every Monday
- cron: "0 0 * * MON"
pull_request:
branches: [ master ]
push:
branches: [ master ]
# Publish semver tags as releases.
tags: [ 'v*.*.*' ]
workflow_dispatch:

env:
REGISTRY: ghcr.io

jobs:
build:
name: Build and publish Taswira
runs-on: ubuntu-latest
permissions:
contents: read
packages: write

steps:
- name: Checkout repository
uses: actions/checkout@v2

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v1

- name: Log into GitHub Container Registry
if: ${{ github.event_name != 'pull_request' }}
uses: docker/login-action@28218f9b04b4f3f62068d7b6ce6ca5b26e35336c
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Extract Docker metadata
id: meta
uses: docker/metadata-action@98669ae865ea3cffbcbaa878cf57c20bbf1c6c38
# It will push the image to ghcr.io/moja-global/taswira:master on push
with:
images: ${{ env.REGISTRY }}/moja-global/taswira

- name: Build and push Docker image
uses: docker/build-push-action@ad44023a93711e3deb337508980b4b5e9bcdc5dc
with:
context: .
# Prevent push to GHCR on pull requests
push: ${{ github.event_name != 'pull_request' }}
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
# Implement Docker build caching for faster builds
cache-from: type=gha
cache-to: type=gha,mode=max
2 changes: 2 additions & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@ dependencies:
- pylint>=2.5.2
- dash==1.13.3
- dash-leaflet==0.0.19
- markupsafe<2.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why these two new dependencies? Where are they used?

Copy link
Member Author

Choose a reason for hiding this comment

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

Without these dependencies, the build fails. Reference: https://mojaglobal.slack.com/archives/C01113X44FR/p1646205676283029

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I see, but not sure why this issue didn't occur in my machine.

Anyway, can you pin jinja2 to an absolute version number?

- jinja2
Loading