jhuckaby / Cronicle

A simple, distributed task scheduler and runner with a web based UI.
http://cronicle.net
Other
3.7k stars 382 forks source link

HTTP Plugin does not allow Variable Subsitution #385

Open arthurvanduynhoven opened 3 years ago

arthurvanduynhoven commented 3 years ago

Summary

When doing [/env/ENV_VAR] it is not replaced.

Steps to reproduce the problem

In URL, Headers, or Post data specify a substitution variable [/env/ENV_NODE]

Your Setup

Running inside Docker

Operating system and version?

Node LTS (Ubuntu)

Node.js version?

Latest LTS

Cronicle software version?

Latest

Are you using a multi-server setup, or just a single server?

both scenarios

Are you using the filesystem as back-end storage, or S3/Couchbase?

no

Can you reproduce the crash consistently?

yes

Log Excerpts

Other Notes:

I will create a MR for fix

mikeTWC1984 commented 3 years ago

I guess it's a good idea to have env variable for that purpose. However the PR you propose will brig a braking change because you are removing job info. I believe there is already env variable carrier - job.env property. You can set it via config, from job_env key. I think the better approach for your ask is to merge job.env (if exist) with process.env.

arthurvanduynhoven commented 3 years ago

I'm new to Cronicle - so spend too much time trying to get env vars working and started to debug the process itself.

So if merging the env context is better and will keep things backwards compatible then we can go that route. I'll close the existing MR.

mikeTWC1984 commented 3 years ago

Whoever needs this functionality, you can add the line below in the begging of the callback on url-plugin.js:

job.env = Tools.mergeHashes(process.env, job.env);