Skip to content
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

A couple fixes and addons #6

Closed
wants to merge 6 commits into from
Closed

Conversation

nodekra
Copy link

@nodekra nodekra commented Nov 7, 2016

  1. child_process.execFile bin/deploy.sh as root if "sudo:true" and invoke it by itself with "sudo -H"
  2. added "group" option, so execFile or sudo will run with $user:$group, instead of $user:root
  3. pass $path and $branch to user task script for better deploy automation: "start to run shell script: sh $task $path $branch"
  4. moved log to /var/log dir

@mytharcher
Copy link
Owner

Thanks so much for improving this tool. There are some suggestion and question on this PR:

  1. Not to change the indent when you just want to modify the logic. Or it will be hard to review the diff.
  2. The feature of user task script I planned to deprecate it. Because by using the .git/hooks/post-merge script in any repository will be a better practice (noticed in Should not support shell command in config file #5 ).
  3. I am not sure what scenario will you use the extraArg?

The feature about root user and group options looks great, I will merge this PR after your fixes.

Cheers

@nodekra
Copy link
Author

nodekra commented Nov 8, 2016

Hi there,

  1. I'm sorry for such mistakes, my first PR and I did changes within github editor with copy-past of whole code from console. I have issues with git inside my IDE, will fix it.
  2. looks correct, I will do it
  3. I use few options for one branch, in my case it is:
    • "origin/dev" {"path": "/var/www/html" }
    • "origin/dev/composer" {"path": "/var/www/html", "shell:" "bin/update_composer.sh", "sudo:" true }

1st will do git fetch only, 2nd will fetch changes as well as run additional task.
So we can do POST request to one branch, but with different tasks localhost:6060/project/id/dev or localhost:6060/project/id/dev/composer . Howere I have done this logic inside user task script, but I think this extraArg still can be usefull.

I will back later with fixes. Thank you.

@mytharcher
Copy link
Owner

Yes. I have same problem as you. Sometimes we need to update composer/npm packages, but we don't want it run every time unless it is updated for saving task time. And I found a useful gist:sindresorhus/7996717 there, for checking to run tasks base on updated items.

I think hookagent should just play a more common role as receiving post hook request and trigger the code update. But after new code updated, what tasks to be run should depend on each project.

And now I realized the parameters in post hook request is also a bad design. Because anyone will add only one post hook for a project to a server. Whatever the remote/branch parameters are, there can be only one, or there will be duplicated requests. A better way to avoid this while we want to make different branches to update their paths is, to analyze which branch/commit come from in the post data. This is also a thought in my mind but have no time to implement.

I think the parameters in post request URL should also be deprecated in some day. But in config file it could be kept for different paths. So, for the extraArg we should not waste more time on it.

@mytharcher
Copy link
Owner

Close due to deprecating feature and long time no updated.

@mytharcher mytharcher closed this Nov 29, 2017
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.

2 participants