Closed flaviouk closed 3 years ago
Hey @flaviouk, I finally found some time to test your PR. It is also working for me, but sure the maintainers cannot merge this change, as it is more of a proof of concept than a merge ready feature. But the needed changes shouldn't be too big.
@aschmolck Is there any reason, that you are using Git over SSH instead of HTTPS? While thinking about the pros and the cons of both, it seems to me that one has to provide the access key either way. Why not use it for cloning instead of specifing and extra SSH key?
Also HTTPS should be available everywhere (also on self hosted platforms), whereas cloning via SSH might not be available. If using HTTPS cloning only is an valid option, we could drop all SSH config and use auth tokens only.
What do you think?
Is there any reason, that you are using Git over SSH instead of HTTPS? While thinking about the pros and the cons of both, it seems to me that one has to provide the access key either way. Why not use it for cloning instead of specifing and extra SSH key?
fwiw, I'm sure it's just historical reasons: the project started as a quick hack to solve a team's need at the time, and we were using git+ssh internally, so that's how it got implemented. I have a feeling that there were some issues with gitlab's support for git+https using tokens back then, but I'm probably mixing it with something else.
Hi @flaviouk I took your idea and wrapped some nessecary code around it, to make the feature configurable. I would love to hear your thoughts on this version of HTTPS support: Here is my PR https://github.com/smarkets/marge-bot/pull/283
You can build and test it with the Dockerfile from your branch.
You champ @matthiasbalke! Thanks! We've been running this slightly modified version for a while with no issues btw. One thing we had to do was add a nodejs wrapper to restart the marge bot, in case it dies and add a docker file for deployment, I don't know if you would consider adding a docker file, I personally quite like having it so that it keeps things portable between environments and ease of deployment
Hi @flaviouk thanks for your feedback, I will gather all feedback in my PR.
But regarding restarting, have you tried starting your docker container using docker restart policy always
:
https://docs.docker.com/config/containers/start-containers-automatically/
The Dockerfile is not needed, as this Projekt uses the nix Buildsystem which supports building Docker Images without having docker installed.
For a reference here is how I build the image on our GitLab:
build-image:
stage: build
image: nixos/nix:2.3.6
script:
- nix-build --attr docker-image default.nix
# artifacts must be located within project directory
- cp -a /nix/store/*-docker-image-marge-bot.tar.gz docker-image-marge-bot.tar.gz
artifacts:
expire_in: 1 day
paths:
- docker-image-marge-bot.tar.gz
Afterwards you can load the image using the docker CLI like this
docker load --input docker-image-marge-bot.tar.gz
I think this should be documented in the contributers file.
Didn't know that at all thanks! I think the reason we added a wrapper was because it takes quite a bit of time to clone (large repo) and I think this helped with reducing startup times between failures. I could be wrong, the issue happened a year ago or so, don't remember all the details. Again thanks for taking the time to put this together and explaining nix to me👍🏻
Closing this in favor of #283
I'll need some guidance to remove the ssh piece..
Build:
docker build -t marge-bot --file Dockerfile .
Run (other flags work as expected):
docker run -it marge-bot --auth-token=YOUR_AUTH_TOKEN