later icon indicating copy to clipboard operation
later copied to clipboard

add later_recurring

Open r2evans opened this issue 3 years ago • 7 comments

Import the functionality of pool::scheduleTaskRecurring directly into later, with additional limits.

This PR is a combination of things I like to do with polling background processes. I'm familiar (a little) with coro, I think this addition fits better here in later than there.

Impetus: I have a bulk-query function that spawns a new process (processx calling sqlcmd) that takes several minutes (data volume, not unexpected). As an optional courtesy in my bulk-query function, I poll the process state and, when exited, I display a message on the console (prompting the user to do something with the new data).

So I do something akin to:

proc <- processx::process$new(...)
notify <- function() message("bulk query complete")
later_recurring(function() {
  alive <- proc$is_alive()
  if (!alive) notify()
  alive # returning logical FALSE results in self-cancellation
}, delay = 10, limit = 100)

Having a never-ending recurring loop might be problematic (and concerning), so there are two additions to pool::scheduleTaskRecurring:

  • self-cancelling func: if the udf returns FALSE, then it is self-cancelling; if nothing (or not the literal FALSE) is returned, it will not self-cancel;
  • limit=NA (the default) allows limitless execution, setting limit=9 permits only 9 iterations before self-cancelling.

r2evans avatar Jul 06 '20 15:07 r2evans

I believe the routine failure is due to GH actions and nothing to do with the patch itself. That is, I'm not sure why the action is trying to push into my fork for this PR:

Collecting information about PR #133...
git push ***github.com/r2evans/later.git -q HEAD:add/later_recurring
remote: Permission to r2evans/later.git denied to github-actions[bot].
fatal: unable to access 'https://github.com/r2evans/later.git/': The requested URL returned error: 403
Error: The process 'git' failed with exit code 128

@wch, is there something I need to do to the PR to fix that failure?

r2evans avatar Nov 30 '21 16:11 r2evans

@r2evans There is some documentation updates to be made. But the actions should be auto pushing it back. I'm working on a fix for the push issue

schloerke avatar Nov 30 '21 17:11 schloerke

@schloerke if there's a problem that I can help speed-along with a manual push to my repo, let me know ... it seems that this set of GH actions is not behaving to your liking.

r2evans avatar Dec 01 '21 02:12 r2evans

@r2evans The GHA won't be able to push back changes to a forked repo (which is this PR's situation). GitHub says it is security issue when trying to push changes back to a forked repo from the main repo.

I'll be working on a better error message in the morning. I'll tag you when it's ready.

(Please don't fix the doc changes yet so I can continue testing in the morning)

schloerke avatar Dec 01 '21 03:12 schloerke

@r2evans The workflow has been updated. Please let me know if the new error message does not provide enough hints/instructions.

Since the workflow has been updated, please fix any errors it reports. Thank you!

schloerke avatar Dec 01 '21 17:12 schloerke

Sorry @schloerke , I do not understand.

New commits:
+ 41b5c492b3f9dfb718b1e1ec03fa43665f54b01f `devtools::document()` (GitHub Actions)
Error: Some auto-generated commits were found. GHA is not allowed to push commits back to a forked repo.
Error: Please perform and commit these actions manually.
Error: Process completed with exit code 1.

While I understand that something in the routine GHA is internally updating the package documentation, why does that need to be pushed back to my branch in order to validate a PR here? It makes no sense to me.

r2evans avatar Dec 01 '21 17:12 r2evans

Why does [the documentation changes] need to be pushed back to my branch in order to validate a PR here? It makes no sense to me.

** Growing pains, sorry.

The RoxygenNote field is getting updated from 7.1.0 to 7.1.2 in the DESCRIPTION file. You are correct in that those changes are not necessary for a valid R CMD check.

If you merge the latest from main (or rebase), the error should go away. Feel free to ignore it as well.

schloerke avatar Dec 01 '21 19:12 schloerke