Add Dockerfile and Github Action #4
Conversation
This will allow CI for creating Docker images on push. Should be changed from push to pull_request once we feel more comfortable with the solution.
erancx
changed the title
This will allow CI for creating Docker images on push. Should be changed from push to pull_request once we feel more comfortable with the solution.
This will allow CI for creating Docker images on push. Should be changed from push to pull_request once we feel more comfortable with the solution.
This will allow CI for creating Docker images on push. Should be changed from push to pull_request once we feel more comfortable with the solution.
| @@ -0,0 +1,30 @@ | |||
| name: Build Docker image | |||
|
|
|||
| on: [push] | |||
There was a problem hiding this comment.
I'd think we'd only want images pushed when tags are created, not on every push, WDYT?
Assumptions
Once #3 is merged, we will have the start of unit tests for this library. The repo will have PRs required via branch protections on the main branch with one approved review. This can be enforced in the pyth-network/pyth-terraform repo.
Developer Submitting A New Change
Developer workstation --> Push to forked repo --> Create a PR against this repo
Some actions run that do pip install -e '.[testing]' and then pytest run to ensure there are no regressions. An action like this one would show test coverage as a comment on the PR.
The image created for running the unit tests is ephemeral and will not be pushed to docker hub. This means the release images will not have extra dependencies for running tests, as that is unnecessary.
Developer Making A New Release
The developer doing this must have Write privileges to this repo.
Developer workstation--> git push --tags --> the below actions run, create an image, and push it to docker hub.
In the future, we can have a required ChangeLog, along with actions to create a new release that gets created here on github AND pushed to pypi. The magical command to create the python source release is python setup.py sdist
There was a problem hiding this comment.
- Regarding the push, this PR is a WIP and I intend to change it to on submit only.
- We would need to different tasks, one for running tests and following by image build. They will be triggered on branch changes only
- We don't have branch protection on our Github account, we'll need to upgrade it for using protection but definitely a requirement for us!
I completely agree with all your notes. I'll make sure to adjust our code to reflect that.
| with: | ||
| platforms: linux/amd64 | ||
| push: true | ||
| tags: ${{ secrets.DOCKERHUB_USERNAME }}/pyth-client-py:latest |
There was a problem hiding this comment.
And when we have this done via tags, we can have an action to just create the docker image with the same version as the library.
There was a problem hiding this comment.
yeah, actually this PR is only a WIP, was looking to get it built and then create a proper k8s deployment. but note taken, this should definitely be an application version rather than tag.
Co-authored-by: Jeff Schroeder <jschroeder@jumptrading.com>
This will allow CI for creating Docker images on push.
Should be changed from push to pull_request once we feel more
comfortable with the solution.