task icon indicating copy to clipboard operation
task copied to clipboard

Add task hooks

Open tylermmorton opened this issue 2 years ago • 3 comments

This PR is in response to a series of issues: #141, #204 and #344. This code adds "hooks" as a feature, with the initial implementation containing 5 hooks. I've added unit tests to cover all of the new behavior, and additional documentation containing examples of each hook!

  • before_all commands are called before a task starts execution
  • after_all commands are called after a task completes, success or failure
  • on_success commands are called when a task completes successfully
  • on_failure commands are called when a task fails due to an exit code other than 0
  • on_skipped commands are called when a task is skipped due to status, precondition or checksum

Here is an example of the usage:

version: '3'

tasks:
  notify:
    vars:
      STATUS: "NONE"
    cmds:
      - curl -d "status={{.STATUS}}" -X POST http://webhook.example.com
      
  test:
    cmds:
      - go test
    hooks:
      on_success:
        - task: notify
          vars:
            STATUS: "PASS" 
      on_failure:
        - task: notify
          vars:
            STATUS: "FAIL"

tylermmorton avatar Jan 29 '22 21:01 tylermmorton

I'm running into issues in VS Code where the Taskfile YAML Schema is complaining about the new hooks field. Any ideas on how to update this? image

Edit: Nevermind, found this ... I will link the pull request here when done

tylermmorton avatar Jan 29 '22 22:01 tylermmorton

Hi @tylermmorton, thanks for yet another PR.

I see future in this PR, but I understand we need to discuss what we really want to do here before proceeding.

The idea of wrapping setting like this into hooks is probably interesting, making it consistent with each other and easy to add more in the future.

The on_failure and on_skipped hooks are likely very useful.

The before_all and on_success feels not needed to me initially, since one could just add more commands to the beginning or end of a task.

In a similar manner, the after_all duplicates functionality we recently released with the defer keyword, so I'd be inclined to remove this one as well.

Regarding #204 you linked, the idea of a possible setup: keyword is that it would be global, so it would kinda run before every declared task. We could consider turning that into a kind of hook, but that would means we would have two kind of hooks, global and local (task-specific) ones.

EDIT: Still about #204, not only running before, and also keeping state like declared functions and environment variables, etc. that would still be accessible inside tasks. So this is actually more than just a hook.


This is the kind of feature where the input from user is very important. So if whoever is reading this would like to add thoughts, they're more than welcome!

andreynering avatar Jan 31 '22 02:01 andreynering

Hey @andreynering, thanks the feedback! :)

I'd like to make the argument to keep after_all as a hook because of the way that defer works. There's some nuance there that isn't completely apparent to users not familiar with Go. To me, users that are already using other hooks to run commands are naturally going to reach for after_all instead of defer.

I agree that before_all and on_success are mostly redundant hooks and I only added them to give the feature some symmetry. If there's an on_failure hook it seems natural to me to have an on_success hook as well. Users defining tasks using hooks might appreciate the semantics that even these redundant hooks provide.

I will say to implement and maintain a hook is much easier than other features, as a hook in the current implementation does not control the flow of or branch any of the task logic. Every new hook definition is roughly 12 lines of code, which could be reduced, especially all of the boilerplate within executor's hooks.go. The call to a hook is just a one-liner!

--

I just read deeper into #204 and I guess I did not get far enough into the discussion to realize this PR does not cover that 😅 My bad. I do really like the idea of a global setup hook that can modify the environment tasks are run in. I'm actually trying to solve this exact problem on one of my projects and the idea of using source and a setup script to build the shell environment seemed like a great solution. There's some value in being able to do that within the Taskfile to help keep all of my team's IaC centralized.

I can probably get some code in for this next time I'm able to pull up my IDE. I will take any notes I have to that issue thread instead of this PR. Would it be easier for you to fold that feature into this branch or should I add the implementation separately?

tylermmorton avatar Feb 02 '22 18:02 tylermmorton