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

(feat): allow fail, timeout, retry, always run tasks #9

Closed
lukasmrtvy opened this issue Jul 22, 2020 · 12 comments · Fixed by #14 or #15
Closed

(feat): allow fail, timeout, retry, always run tasks #9

lukasmrtvy opened this issue Jul 22, 2020 · 12 comments · Fixed by #14 or #15
Labels
feature New feature to request

Comments

@lukasmrtvy
Copy link

lukasmrtvy commented Jul 22, 2020

Would be nice to have possiblity to add:

options to tasks.

    tasks:
    - name: pre
      task:
      - func: shell
        do:
        - printf 'Prepare Env'
       always: pre
    - name: task
      task:
      - func: shell
        do:
        - sleep 60
       timeout: 10
       retries: 3
       allow_fail: true
       allow_exitcodes: 
       - 128
    - name: post
      task:
      - func: shell
        do:
        - printf 'Cleanup Env'
      always: post

Thanks

@stephencheng stephencheng added the feature New feature to request label Jul 23, 2020
@stephencheng
Copy link
Contributor

To address these one by one:

  • timeout

Do you have a supporting case?

What I can think of this is in the case of a synchronised call, the server end is taking long time to process.

In this case, it should fall back to how the client tool is talking to the server, eg,

curl https://fantatic.resource.com/myfile

The design of UPcmd is to try to collaborate and try to simplify the usage of multiple threads etc, I have got the thoughts here: https://upcmd.netlify.app/ideas_and_faq/concurrency/

It is technically possible and it is a strength of golang, but do we need it?

Continue above case, the responsibility should be really the client tool to deal with the timeout, rather than UPcmd.

Use this in shell func, problem should be solved:

curl --max-time 5.5 https://example.com/

Aslo see the recommend tools

  • retry

No need an implementation, the loop cmd + sleep cmd will do

https://upcmd.netlify.app/cmd-func/c0087/

You might need to generate an array for the time being as it requires that to iterate through, let me know if you like some thing like while without specifying the loop var, if so please raise as a feature

  • allow_fail

I don't get this, could you explain a little bit more?

Currently, you can ignoreError, it actually allows to fail and continue, see https://github.com/upcmd/up/blob/master/up.yml, I use it to ignore not supported build and move on

  • allow_exitcodes

No change required for this, you can achieve this by:

  1. ignoreError
  2. capture the result.Code then evaluate it to do whatever you like
  • always run post task and pre task

I have this in the plan already, would you mind creating this as a feature

Thanks

@stephencheng
Copy link
Contributor

@lukasmrtvy PS. what's the purpose of the pre-task? I could understand the always run post-task is to clean up resources usage, close file handler etc, why do we need pre-task? Any use case?

Would below be okay?

tasks: 

- name: Main
  task:
    - func: call
       do:
         - pre-task
         - my-task-to-do-business
         - post-task
  
- 
  name: pre-task
  task:

- 
  name: post-task
  task:

- 
  name: my-task-to-do-business
  task:

@stephencheng
Copy link
Contributor

@lukasmrtvy Also as to the link you mentioned: https://docs.gitlab.com/ee/ci/yaml/#yaml-anchors-for-before_script-and-after_script

I am aware of using yaml anchors to do some fancy stuff and this has taken into consideration during the design phase, I find the anchor is a little hard for lots of users, the syntax will just challenge many users and this is actually the goal of the project of UPcmd to lower the bar to become more friendly to be adopted.

Having said that, the pre-task and post-task are possible to be implemented in a better/easier way to end users

For your above use case, do check out the doc site: https://upcmd.netlify.app/ and follow my suggestion above, if you still find challenging, I am happy to help you prototype it.

@lukasmrtvy
Copy link
Author

Hello :)

* timeout

Do you have a supporting case?

Well, it can be achieved with GNU timeout, I am not aware of POSIX alternative.
Lets say that I will invoke az login or aws sso login:

$ aws sso login --profile my-dev-profile
Using a browser, open the following URL:
 
https://my-sso-portal.awsapps.com/verify

and enter the following code:
QCFK-N451

CLI will stay open for some "unknown" time. I am not aware of any timeout option to aws cli.
Timeout is something generic, that can end task after some specified time.

* retry

No need an implementation, the loop cmd + sleep cmd will do

https://upcmd.netlify.app/cmd-func/c0087/

Ok, but still, it plays nicely with possible timeout option..

* allow_fail

I don't get this, could you explain a little bit more?

Currently, you can ignoreError, it actually allows to fail and continue, see https://github.com/upcmd/up/blob/master/up.yml, I use it to ignore not supported build and move on

Sorry, I did not notice. ignoreError is answer in this case.

* allow_exitcodes

No change required for this, you can achieve this by:

1. ignoreError

2. capture the result.Code then evaluate it to do whatever you like

Ok, its just a proposal. I like how Ansible does these type of things..

* always run post task and pre task

I have this in the plan already, would you mind creating this as a feature

Superb. I will. Post task is definitely a must have.

@stephencheng
Copy link
Contributor

@lukasmrtvy I believe there are timeout settings for aws cli, at least for individual sub command

A simple google of timeout finds this, FYI


--cli-read-timeout (int)

The maximum socket read time in seconds. If the value is set to 0, the socket read will be blocking and not timeout.

--cli-connect-timeout (int)

The maximum socket connect time in seconds. If the value is set to 0, the socket connect will be blocking and not timeout.

@lukasmrtvy
Copy link
Author

yes, but for example az login does not have any configurable timeouts..

@stephencheng
Copy link
Contributor

Yeah, you are right, I think it is fair to have a timeout mechanism

@stephencheng
Copy link
Contributor

To conclude of this thread:

Below features are considered

  • optional timeout for shell func
  • optional always run post task feature, similar to what Ansible does

@stephencheng
Copy link
Contributor

stephencheng commented Jul 28, 2020

@lukasmrtvy

The retry should now fully work, it's in latest release:

curl -s https://api.github.com/repos/upcmd/up/releases \
    |grep darwin_amd64_latest \
    |grep download \
    |head -n 1 \
    |awk '{print $2}' \
    |xargs -I % curl -L % -o up \
    && chmod +x up

See Examples:

https://github.com/upcmd/up/blob/master/tests/functests/c0167.yml
https://github.com/upcmd/up/blob/master/tests/functests/c0168.yml
https://github.com/upcmd/up/blob/master/tests/functests/c0169.yml

      -
        func: block
        desc: |
          an example of retry
          it tries to ping 100 times with a timeout of 2 seconds every time
          with every attempt it puts a delay of 2 seconds
          it also uses dynamic loopRange instead of prepared list of value
        loop: '{{loopRange 1 100 "myretry"}}'
        do:
          -
            func: shell
            do:
              - ping goole.com
            timeout: 500
            flags:
              - ignoreError
          -
            func: cmd
            do:
              - name: sleep
                desc: delay 2 seconds in each attemp
                cmd: 2000

Can you try it? and let me know how it goes

@stephencheng
Copy link
Contributor

@lukasmrtvy

Added the finally block support to close/clean up resources, also support a rescue element to deal with whether you'd like to suppress the error code to be non 0 return

https://github.com/upcmd/up/blob/master/tests/functests/f0171.yml
https://github.com/upcmd/up/blob/master/tests/functests/c0172.yml
https://github.com/upcmd/up/blob/master/tests/functests/c0173.yml
https://github.com/upcmd/up/blob/master/tests/functests/c0174.yml
https://github.com/upcmd/up/blob/master/tests/functests/c0175.yml

I will update the docs a little bit further, but you should be able to follow examples

Please let me know if this could help with your use case

@stephencheng
Copy link
Contributor

@lukasmrtvy it's now in latest version or latest rolling release binary

@stephencheng stephencheng pinned this issue Aug 1, 2020
@stephencheng
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature to request
Projects
None yet
2 participants