Skip to content
This repository has been archived by the owner on Sep 20, 2022. It is now read-only.

[HIVEMALL-84-3] Update docker-compose.yml and documents #81

Closed
wants to merge 4 commits into from

Conversation

takuti
Copy link
Member

@takuti takuti commented May 18, 2017

What changes were proposed in this pull request?

I tried things written in the Docker on Hivemall documentation and noticed a couple of things we can improve.

  • Update docker-compose.yml to behave in a similar manner to docker run
  • Update Docker on Hivemall documentation

What type of PR is it?

Documentation

What is the Jira issue?

https://issues.apache.org/jira/browse/HIVEMALL-84

How was this patch tested?

Manually

@takuti
Copy link
Member Author

takuti commented May 18, 2017

@amaya382 could you check this if you get a chance?

@coveralls
Copy link

coveralls commented May 18, 2017

Coverage Status

Coverage remained the same at 38.694% when pulling e071eda on takuti:docker into 10e7d45 on apache:master.

Copy link
Member

@amaya382 amaya382 left a comment

Choose a reason for hiding this comment

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

let me set up dockerhub?

3. `docker run -p 8088:8088 -p 50070:50070 -p 19888:19888 -it hivemall/latest:20170517`
Refer [Docker reference](https://docs.docker.com/engine/reference/run/) for the command detail.

Similarly to the `volumes` option in the `docker-compose` file, `docker run` has `--volumes` option:
Copy link
Member

Choose a reason for hiding this comment

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

typo --volume. as an other choice, -v would be easier


You can find pre-built Hivemall docker images in [this repository](https://hub.docker.com/r/hivemall/latest/).

1. Check [the latest tag](https://hub.docker.com/r/hivemall/latest/tags/) first
Copy link
Member

Choose a reason for hiding this comment

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

This repository is in the wrong way, must be fixed its name as hivemall and its tag as version ({owner}/{repository}:{tag}, like hivemall/hivemall:latest or hivemall/hivemall:0.4.2-rc.2).
And if distribute image via dokcerhub, we should prepare automated build to retain consistency and integrity.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I see. This part is written by @myui in commit 10e7d45.

Hi @myui, could you make further discussion in here or another issue? I guess this part was temporarily necessary for the BoF session of the Apache BigData conf.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or, if @amaya382 can set up this, it would be nice after this PR has been merged.

Copy link
Member

@myui myui May 20, 2017

Choose a reason for hiding this comment

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

owner is used for repository.
repository is used for branch.
tag is used for versioning.

hivemall:release:v0.5-rc.1
hivemall:lastest:20170502-1
hivemall:spark:20170501

owner: hivemall is used only for hivemall. hivemall:hivemall is redundant. So, okey as it is.

Copy link
Member

Choose a reason for hiding this comment

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

It's redundant but depart from docker's usual practice...
e.g.

  • automated build uses branch or tag (on git) as tag (on docker)
  • if become an official repository or under apache, its name will be broken

Copy link
Member

Choose a reason for hiding this comment

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

I'll ask for ASF INFRA to get an access to https://hub.docker.com/u/apache/

Copy link
Member

Choose a reason for hiding this comment

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

@amaya382 @takuti official Dockerhub can only be used for official Apache releases.

See discussions in
https://issues.apache.org/jira/browse/INFRA-14123?jql=project%20%3D%20INFRA%20AND%20text%20~%20dockerhub

Unofficial docker images better not be advertised officially...

Copy link
Member

Choose a reason for hiding this comment

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

It seems better to distribute TAGGED images on docker hub (apache/hivemall) by automated build and distributing NOT TAGGED ones should depend on manual build from Dockerfile.
(The former for evaluation by user, the latter for developer)

http://www.apache.org/dev/release-distribution.html#unreleased

Copy link
Member

@myui myui May 22, 2017

Choose a reason for hiding this comment

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

We can only publish TAGGED image associated with ASF official release. Release for each commit is not allowed in ASF distribution policies.


> #### Note
> You can [skip](./getting_started.html#running-pre-built-docker-image-in-dockerhub) building images by using existing Docker images.
> You can [skip](./getting_started.html#running-pre-built-docker-image-in-dockerhub) building images by using a pre-build docker image from Docker Hub.
Copy link
Member

Choose a reason for hiding this comment

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

...-in-docker-hub ?

@takuti
Copy link
Member Author

takuti commented May 20, 2017

@amaya382 Fixed document, thanks!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 38.694% when pulling 1dd46e2 on takuti:docker into 10e7d45 on apache:master.

1 similar comment
@coveralls
Copy link

coveralls commented May 20, 2017

Coverage Status

Coverage remained the same at 38.694% when pulling 1dd46e2 on takuti:docker into 10e7d45 on apache:master.

@takuti
Copy link
Member Author

takuti commented May 23, 2017

Even only for release, it sounds nice. Let's try it in the next (first) apache release if possible :)

For now, I've updated the description of the Docker Hub image. I keep the part, but mentioned that the distributed image might be out-of-data. Is it okay for you @myui @amaya382 ?

In terms of naming, it should be re-considered when we officially distribute an image according to ASF release.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 39.027% when pulling 9e9227b on takuti:docker into 10e7d45 on apache:master.

@coveralls
Copy link

coveralls commented May 23, 2017

Coverage Status

Coverage remained the same at 38.694% when pulling 9e9227b on takuti:docker into 10e7d45 on apache:master.

@amaya382
Copy link
Member

amaya382 commented May 23, 2017

@takuti looks good!
Once close HIVEMALL-83 and let's reconsider in other issue/PR

@amaya382
Copy link
Member

Created https://issues.apache.org/jira/browse/HIVEMALL-106

@takuti
Copy link
Member Author

takuti commented May 29, 2017

@myui ^ everything is okay?

@myui
Copy link
Member

myui commented May 29, 2017

will check soon.

@asfgit asfgit closed this in 492b5d8 Jun 6, 2017
@myui
Copy link
Member

myui commented Jun 6, 2017

LGTM. Merged to master. Thanks!

@takuti takuti deleted the docker branch September 20, 2017 07:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants