Skip to content

Conversation

@felixbouleau
Copy link

Adds a /start ping to Healthchecks.io to help debug stuck jobs and measure execution time.

I've tested this both with and without $CHECK_URL set and it worked fine. Hopefully it can be useful for someone else.

Thanks @bcardiff for the great image!

@bcardiff
Copy link
Owner

Thanks @felixbouleau, in order to keep users unlocked from healthchecks.io, WDYT about

  1. using a START_URL argument
  2. setting START_URL to CHECK_URL/start by default in the entrypoint (if START_URL is not set and CHECK_URL it is set)

something like:

  if [ -z ${START_URL+x} ]
  then 
    if [ -z ${CHECK_URL+x} ]
    then;
    else
      START_URL="${CHECK_URL}/start"
    fi
  fi

why -z ${var+x}?

  1. use the same code for CHECK_URL and START_URL. (I'm not sure if checking with -n is better than -z here)
  if [ -z "$START_URL" ]
  then
    wget $START_URL -O /dev/null
  fi
  1. Add a description in the README

?

The behavior should be the same and the user can opt-out the START_URL or define it's own.

FYI #7 attempt to implement FAIL_URL, I need to do another round and merge that functionality. But is related and you might be interested.

@felixbouleau
Copy link
Author

@bcardiff thanks for the quick feedback!

Since the goal was to be agnostic towards ping providers I've taken a slightly different approach than #7 for defaults. Here's a shortcut to the logic.

This way, if you use something else than HC you don't need to override START_URL with something else.

I'm happy to switch over to the #7 approach if you prefer it - just let me know. It's a super easy change. :)

@bcardiff
Copy link
Owner

It's fine as is. Thanks!

The duplicated wget bugs me a little. But is fine.

Bash is always cryptic to me.

  • Was there any reason to drop the proposed +x? Just curious.
  • The other checks use [ -z "$VAR" ] while the PR is using [ -z ${VAR} ] (as a consequence of dropping +x.
  • "${VAR}/start" is the same as "$VAR/start"

I'm not asking for additional changes, but highlighting the difference with the other checks for comparison and eventual unification.

@felixbouleau
Copy link
Author

@bcardiff Dropping the -x was a knee jerk fix for a bug I introduced. I solved it in a better way now. Basically I included START_URL as an ENV in the Dockerfile which sets it to an empty string. So the [ -z ${VAR+x} ] evaluated to x and broke the if statement. Instead of dropping the -x I dropped the ENV in the Dockerfile.

dantebarba pushed a commit to dantebarba/docker-rclone that referenced this pull request Jul 14, 2023
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