Skip to content

Conversation

@yajo
Copy link
Contributor

@yajo yajo commented Dec 29, 2016

Hi friends, I found this to be very useful.

We'd love to be able to use this tool in docker environments, or simply in localhost without having to blur local system with pip installs somewhere.

This simple patch adds support for it. Of course, besides merging this, you'd have to create an automated build in your Docker Hub, and then replace lines referring to tecnativa/git-aggregator image with the brand new acsone/git-aggregator one. I can help you in the process if you want.

@Tecnativa

MAINTAINER Tecnativa <info@tecnativa.com>
ENTRYPOINT ["/usr/src/app/entrypoint.sh"]
ENV GIT_AUTHOR_NAME=git-aggregator \
EMAIL=https://hub.docker.com/r/tecnativa/git-aggregator
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once you set up your automated build, you'd have to refer to https://hub.docker.com/r/acsone/git-aggregator here.


You can use `git-aggregator` from CLI without installing anything but Docker:

$ docker run -it --rm -v $(pwd):/repos tecnativa/git-aggregator -c repos.yaml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

... and to acsone/git-aggregator here.

@coveralls
Copy link

coveralls commented Dec 29, 2016

Coverage Status

Coverage remained the same at 88.104% when pulling 3b26eb1 on Tecnativa:master-docker into d2782cf on acsone:master.

@sbidoul
Copy link
Member

sbidoul commented Dec 29, 2016

@yajo thanks for your interest and contribution!

git-aggregator is a pure python library and CLI. There are many ways to install it and possibly isolate it from the system. docker is just one of them. virtualenv is another possibility and there are more. For instance, if you don't want to pollute your system with a global pip install, you can install it in a virtualenv and symlink the executable in your PATH, eg:

$ virtualenv /opt/git-aggregator-venv
$ /opt/git-aggregator-venv/bin/pip install git-aggregator
$ ln -s /opt/git-aggregator-venv/bin/gitaggregate /usr/local/bin/gitaggregate

So I tend to think adding docker support goes a bit beyond the scope of this project.

@yajo
Copy link
Contributor Author

yajo commented Dec 29, 2016

Although virtualenv served its purpose for many years, the isolation it provides is way behind Docker's. I think the work is very little (I did most of it for you 😉) and the benefits are a lot. In fact I think adding Docker support is good in any project! A couple of files and some configuration and suddenly you have an automatically up-to-date tiny pluggable operating system that flawlessly runs your app, awesome! 😊

This enables any DevOps to plug the library into any dockerized environment by simply sharing a volume, or running in localhost by a new CLI option that he knows includes every possible dependency and handles every corner case.

OTOH it's not a matter of if this is going to be dockerized or not. It is already, and will continue being. It is a matter of sharing efforts or forking them. I'm pretty sure we all believe sharing is better for everybody. ☺️

@coveralls
Copy link

coveralls commented Jan 2, 2017

Coverage Status

Coverage remained the same at 88.104% when pulling 9bb092d on Tecnativa:master-docker into d2782cf on acsone:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 88.104% when pulling 9bb092d on Tecnativa:master-docker into d2782cf on acsone:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 88.104% when pulling 4907937 on Tecnativa:master-docker into d2782cf on acsone:master.

1 similar comment
@coveralls
Copy link

coveralls commented Jan 2, 2017

Coverage Status

Coverage remained the same at 88.104% when pulling 4907937 on Tecnativa:master-docker into d2782cf on acsone:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 88.104% when pulling c414ade on Tecnativa:master-docker into d2782cf on acsone:master.

1 similar comment
@coveralls
Copy link

coveralls commented Jan 2, 2017

Coverage Status

Coverage remained the same at 88.104% when pulling c414ade on Tecnativa:master-docker into d2782cf on acsone:master.

@coveralls
Copy link

coveralls commented Jan 2, 2017

Coverage Status

Coverage remained the same at 88.104% when pulling d586b90 on Tecnativa:master-docker into d2782cf on acsone:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 88.104% when pulling d586b90 on Tecnativa:master-docker into d2782cf on acsone:master.

@coveralls
Copy link

coveralls commented Jan 2, 2017

Coverage Status

Coverage remained the same at 88.104% when pulling 323056e on Tecnativa:master-docker into d2782cf on acsone:master.

@coveralls
Copy link

coveralls commented Jan 2, 2017

Coverage Status

Coverage remained the same at 88.104% when pulling dc06d92 on Tecnativa:master-docker into d2782cf on acsone:master.

@coveralls
Copy link

coveralls commented Jan 3, 2017

Coverage Status

Coverage remained the same at 89.007% when pulling cea11bf on Tecnativa:master-docker into 115f7e9 on acsone:master.

@yajo
Copy link
Contributor Author

yajo commented Jan 24, 2017

Here you can see how I'm using it right now:

This is useful to deploy a fully docker-based devel env.

@lasley what do you think?

Copy link
Contributor

@lasley lasley left a comment

Choose a reason for hiding this comment

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

I agree that Dockerization is something that benefits all projects greatly. I am definitely more likely to test a project out if it is pre-Dockerized vs setting up an environment manually.

The argument could be made that a pip install is pretty easy too. I counter this with the fact that a virtual env setup, activation, pip install, then subsequent deactivation & deletion is a 5-6 step process - not even considering if some sort of development headers are required. My workstations are Mac, so I find myself butting against the dev headers pretty often due to my refusal to install something like homebrew due to the mess it makes of the filesystem.

To contrast this, testing something in Docker is always a 1 step process & that process is always the same. This allows me to skip all installation steps for a repo & go directly to using it. I find this incredibly beneficial when searching for a lib/app to base mine from. Additionally- I absolutely despise reading manuals, so yeah definitely a big win IMO.

MAINTAINER Tecnativa <info@tecnativa.com>
ENTRYPOINT ["/usr/src/app/entrypoint.sh"]
ENV GIT_AUTHOR_NAME=git-aggregator \
EMAIL=https://hub.docker.com/r/tecnativa/git-aggregator
Copy link
Contributor

Choose a reason for hiding this comment

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

Tecnativa repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because this code builds https://hub.docker.com/r/tecnativa/git-aggregator/, and I don't want to address people to Acsone until they acknowledge maintenance of this image (in which case it would become https://hub.docker.com/r/acsone/git-aggregator/ or something similar). That's why I said what I said in #5 (comment).

EMAIL=https://hub.docker.com/r/tecnativa/git-aggregator
# HACK Install git >= 2.11, to have --shallow-since
# TODO Remove HACK when python:alpine is alpine >= v3.5
RUN apk add --no-cache --repository http://dl-cdn.alpinelinux.org/alpine/v3.5/main git
Copy link
Contributor

Choose a reason for hiding this comment

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

@yajo Hrmmmm so remember when I reported that this didn't work in the Odoo Docker container? Well it works here.

I took it a step further and created a dockerfile with just this in it, and it still worked. My curiosity is peaked, so I'll probably fool around with this a bit more in that context.

if [ $uid -ne $(id -u root) ]; then
gid=$(stat -c %g /repos)
addgroup -g $gid threpwood
adduser -G threpwood -u $uid guybrush -DH -s /bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

threpwood & guybrush - Intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely 😆

All that matters is the UID & GID, so I had to put some name and that one came to my mind ☠️

Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, mainly due to my newness in Docker arena- if you use a user that exists on the host system, are the GID and UID matched with the system user/group of the same name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's why this hack, it allows you to use the docker app and let it use your same user permissions. So after cloning, no root-owned files are there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome thanks for the elaboration! I've been running into permissions issues lately & I have no idea why this never dawned on me 😄

@sbidoul
Copy link
Member

sbidoul commented Apr 18, 2017

After further thinking we consider maintaining a docker image is out of scope of this project. People willing to maintain a docker image of git-aggregator are welcome to do so of course. That does not require a fork of this project.

@yajo I see here you install it as part of a larger image so I guess this is less important for you now.

@sbidoul sbidoul closed this Apr 18, 2017
@yajo
Copy link
Contributor Author

yajo commented Apr 21, 2017

Indeed, thanks anyway 😉

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