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

Add Dockerfile and build image flow #567

Merged
merged 17 commits into from
May 28, 2024

Conversation

grieve54706
Copy link
Contributor

@grieve54706 grieve54706 commented May 24, 2024

Description

Add Dockefile and GitHub workflow to build images.

Additional information

This PR uses pull_request labeled event to test. Please ignore the many test label messages.

on:
  pull_request:
    types: [labeled]

We may use the event on.push.branches to test the GitHub workflow in the future.

@grieve54706 grieve54706 added test and removed test labels May 24, 2024
@grieve54706 grieve54706 marked this pull request as ready for review May 24, 2024 10:35
Copy link
Contributor

@goldmedal goldmedal left a comment

Choose a reason for hiding this comment

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

Some minor comments, Others LGTM.

make run
```

## Contributing
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Contributing
## Developer Guide

It's a kind of guide to show how start a development environment for developers.
I think Developer Guide would be better.

docker-build:
docker image build . -t wren-engine-ibis

docker-run:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
docker-run:
dev-docker-run:

How about add a dev prefix? I guess someone maybe confuse on which version would be started by this command. The image on repo? or The local build.
I guess this one will start the local one, right?

Copy link
Member

@paopa paopa left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Comment on lines +54 to +61
- name: Docker meta
id: meta
uses: docker/metadata-action@v5
with:
images: ghcr.io/canner/wren-engine-ibis
tags: |
type=sha
type=raw,value=nightly
Copy link
Member

Choose a reason for hiding this comment

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

First time to know this action to put the image tags. it's really concise!

@goldmedal goldmedal merged commit 5cf9c5c into feature/ibis-server May 28, 2024
@goldmedal goldmedal deleted the feature/ibis/docker-image branch May 28, 2024 05:58
@goldmedal
Copy link
Contributor

Thanks @grieve54706 @paopa

goldmedal pushed a commit that referenced this pull request May 28, 2024
* Add Dockerfile

* Make ibis version same as java engine

* Add CI

Install python dependencies in the CI

* Make python dependencies install in the Docker

If we install the dependencies in the CI, may cause platform issue

* Use docker/build-push-action

* Fix tag name

* Add docker/metadata-action

* Fix Dockerfile path

* Fix context

* Fix tag

* Update README

* Remove invalid default working-directory

* Fix pyproject.toml path

* Use step.working-directory

* Use poetry version

* Add empty line in the end of file

* Edit title
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.

3 participants