-
Notifications
You must be signed in to change notification settings - Fork 27
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice progress! Great that you make it works on the example repository 🎉
There are few changes requested from my side (some of them you have already mentioned in the TODO list of the PR). Take them more like suggestions as it is still WIP.
There is one task left to finish this PR. We need to have installation_id for each installed repository to make access for actions over HTTP (e.g. git push). We have two options:
Can we discuss or vote for one of these options @jpopelka @rpitonak |
We can make regular back-ups to prevent this. |
elif 'installation' in webhook_payload.keys(): | ||
if webhook_payload['action'] == 'added': # detect new repo installation | ||
installation_id = webhook_payload['installation']['id'] | ||
repositories_added = webhook_payload['repositories_added'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't installation ID unique for every org/user? Wouldn't be enough to save {org/username : ID}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find documentation so I am not sure that this will be enough. Therefore, I saved the installation id for every repo. But I can change it can you send me some link to confirm your assumption? @rpitonak
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tested yet, I'll give it a try once it's not WIP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last several notes, sorry to not raising these earlier.
See also user-cont/release-bot-deployment#11 |
I've put the suggested changes into marusinm#1 I've deployed it locally and was able to make release-bot work as Github App, but it still doesn't work as I'd expect, because when I, for example, create an issue all I see in the pod/release-bot is:
and that's all I get, there's nothing else in the log - which makes me wonder where are the other debug logs I'd expect to see. |
Thanks for your changes in marusnm#1 I just merged them. When I check my old deployment logs with INFO debugging mode I can see this and many more.
The annoying is that all logs are printed twice because of the celery worker in the same pod. However, I can see them all. When I look at your log, I guess r-bot detected your new issue via webhook. Then it stopped executing. This is caused when the Github App installation token isn't created correctly. When I find this error for the first time I just had bad workflow which should be the following:
At the moment I can't see another issue. |
Ok, that might be the problem because since I run the cluster locally I need to do some magic ( So I have to believe you it works. |
Ok, I think it's time to merge this. Unfortunately, I've created a conflict by merging #247 Please rebase so that I can merge. |
Totally forgot about this. Merging. Thanks for the outstanding work! 🎆 |
Implementation of necessary stuff like celery tasks, token generation, Dockerfile, run over multiple projects, etc. Deployment for OpenShift is available here.
WIP:
git push
/files
The reference repo with an installed release-bot app is available here. I added you as collaborators @rpitonak @TomasTomecek @jpopelka you can play with it.
PR is related to #119 #112