-
Notifications
You must be signed in to change notification settings - Fork 136
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 https support #248
Add https support #248
Conversation
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? |
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. |
You champ @matthiasbalke! Thanks! We've been running this slightly modified version for a while with no issues btw. |
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 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