Strider-CD / strider-github

Github provider for strider
25 stars 34 forks source link

Delete branch #72

Closed xgalen closed 6 years ago

xgalen commented 7 years ago

Hi,

In our team we need to exec something (with custom-scripts plugin) when deleting a branch. However, since Strider first tries git cloneand fails, the custom-scripts plugin is not executed.

We can check when a branch is deleted with her payload:

created: false,
deleted: true,
forced: true,
...

Is it possible to Strider doesn't do a git clone and continue with the other plugins? If not, do you think it's possible to make some code changes to get it? I could do, of course, if you give some hints. Any suggestions are welcome.

Thanks in advance.

oliversalzburg commented 7 years ago

I didn't even know Strider does anything when a branch is deleted. That makes no sense.

xgalen commented 7 years ago

I mean when you delete a branch on GitHub and if you have configured the event (and we want this, in order to exec some scripts through Strider), GitHub send the event to Strider.

Strider doesn't know if the event is a branch creation, deletion or just update.

The error occured is this (obviously because the branch does not exist):

2016-08-02T10:05:06.525Z - error: Job 57a0704f8df8b67e01b3d07b  Git clone failed with code 128 Error: Git clone failed with code 128
    at badCode (/home/ubuntu/strider/node_modules/strider-git/worker.js:90:11)
    at updateCache (/home/ubuntu/strider/node_modules/strider-git/worker.js:154:19)
    at ChildProcess.<anonymous> (/home/ubuntu/strider/node_modules/strider-simple-runner/node_modules/strider-runner-core/lib/job.js:203:9)
    at ChildProcess.emit (events.js:98:17)
    at Process.ChildProcess._handle.onexit (child_process.js:820:12)
2016-08-02T10:05:06.526Z - info: [runner:simple-runner] Job done with error. Project: mediasmart/api Job ID: 57a0704f8df8b67e01b3d07b
2016-08-02T10:05:06.527Z - info: [runner:simple-runner] Job done without error. Project: mediasmart/api Job ID: 57a0704f8df8b67e01b3d0
knownasilya commented 7 years ago

We could execute cleanup probably..

oliversalzburg commented 7 years ago

Well, the webhook is supposed to be called on pushes to a branch. You can call it for whatever you want, but that's not what it is intended for and, thus, the result you're seeing.

What do you want Strider to actually do when this event happens? Like, if you just want to run some custom script, why even involve Strider in this? It would probably be trivial to write a small Express application that handles this job for you.

xgalen commented 7 years ago

We want to involve Strider on this because the script is to remove things created before by Strider as a part of our CD process. It's a way to close the circle, doing all the process with Strider. It's much robust maintain all of the process on Strider than one things on Strider and others in a small app.

If we could execute cleanup as knownasilya says and continue with the other plugins would be perfect.

knownasilya commented 7 years ago

Another idea would be to add an error phase, which could have STRIDER_JOB_ERROR_MSG and STRIDER_JOB_ERROR_CODE envs. This might be more useful since errors happen for many reasons.

oliversalzburg commented 7 years ago

I dunno, the whole thing seems like an XY problem. If there is something that is performed in Strider which creates persistent information, then Strider should probably clean it up itself.

If someone decides to create persistent information for every branch that is tested, then that's really not a problem Strider should solve.

If we were to introduce an error phase, then that would only enable people to write their own plugins to do something in that case, but then why not create your own plugin in the first place to handle this situation?

If we were to support webhooks for branch deletions, then we would want to support them in all provider plugins and I don't see the point.

knownasilya commented 7 years ago

Really this is the issue that Strider is too narrow in it's handling of provider events (branches only - commit/pr) and falls under the task of "Strider needs environments" https://github.com/Strider-CD/strider/issues/367

xgalen commented 7 years ago

And what about to execute the cleanup phase? This does not create a new phase, does not affect other plugins and add a new feature (support branch deletions).

oliversalzburg commented 7 years ago

Well, I guess it would be a good idea to always run the cleanup phase. I can't see anything wrong with that.

xgalen commented 6 years ago

I think we can close this after the latest update.

knownasilya commented 6 years ago

Thanks for the reminder