search

bnosac / cronR

A simple R package for managing your cron jobs.
Other
288 stars 38 forks source link

cronjobs parse into an extra "desc: tags:"-line when no environment is passed #50

Closed gerhardqux closed 2 years ago

gerhardqux commented 2 years ago

With the introduction of environment options in v0.6.1, parsing seems to be broken. When no environment is passed, an additional empty line is inserted.

> cron_add("w","monthly",days_of_month="15th",ask=FALSE)
Adding cronjob:
---------------

## cronR job
## id:   ca9aaf633981f89df215b31c8e3621bb
## tags: 
## desc: 

0 0 15 * 1 w
> cron_ls()
## cronR job
## id:   ca9aaf633981f89df215b31c8e3621bb
## tags: 
## desc: tags:
## desc: 

0 0 15 * 1 w

This is caused by the if-statement at https://github.com/bnosac/cronR/blob/master/R/cron_add.R#L203 which evaluates to NULL when no environment is given. The NULL is still pasted and separated with an \n character. Hence, the empty line.

jwijffels commented 2 years ago

You are right, there is a newline now in the default setting If I recall well, it doesn't hurt neither when I tested this but maybe I mistested. https://github.com/bnosac/cronR/pull/51 looks ok

gerhardqux commented 2 years ago

I'm fiddling a little with it. It does actually seem to cause some undesired behaviour in cron_rm(), but I'm not really sure why it's only triggered there.

> cron_clear(ask=FALSE)
> cron_add("w","monthly",days_of_month="14th",description="test", ask=FALSE)
Adding cronjob:
---------------

## cronR job
## id:   d5a96a284b96c0a3e4f17e68bba47e17
## tags: 
## desc: test

0 0 14 * 1 w
> cron_add("uptime","monthly",days_of_month="16th",description="test2",ask=FALSE)
Adding cronjob:
---------------

## cronR job
## id:   b1072b582edd064dd8d4ecb6dd591539
## tags: 
## desc: test2

0 0 16 * 1 uptime
> cron_ls();
## cronR job
## id:   d5a96a284b96c0a3e4f17e68bba47e17
## tags: 
## desc: test tags:
## desc: test

## cronR job
## id:   b1072b582edd064dd8d4ecb6dd591539
## tags: 
## desc: test2 tags:
## desc: test2

0 0 14 * 1 w

0 0 16 * 1 uptime

> cron_rm('b1072b582edd064dd8d4ecb6dd591539')
Removed 1 cron job.
Are you sure you want to remove the specified cron job with id 'b1072b582edd064dd8d4ecb6dd591539'? [y/n]: 1: y
> cron_ls();
## cronR job
## id:   d5a96a284b96c0a3e4f17e68bba47e17
## tags: 
## desc: test tags:
## desc: test

0 0 14 * 1 w

0 0 16 * 1 uptime

Warning message:
In desc_start:(length(cronR_job) - 1) :
  numerical expression has 2 elements: only the first used
jwijffels commented 2 years ago

maybe @atusy can do some more testing as well

jwijffels commented 2 years ago

I only tested this with one job. Damn

gerhardqux commented 2 years ago

thanks for the quick response! This has been a very good experience for me.

atusy commented 2 years ago

Sorry for the regression, and thanks @gerhardqux for the quick fix!

jwijffels commented 2 years ago

I've uploaded the package to CRAN for approval by CRAN. The package does not pass the automatic flow from CRAN as it is a unix-only package but if all goes well, the fix should be on CRAN soon.

jwijffels commented 2 years ago

The updated package is now on CRAN. Thanks again @gerhardqux for the fix(es)