Mrc 5114 Add a submit function to the queue object #5

M-Kusumgar · 2024-03-07T16:26:18Z

Finally got a PR up for this! This adds the submit function to the queue and returns the task_id from rrq. The strategy for the worker dirs was that we were going to git clone the orderly root and fetch before we run a report. We can then checkout and hard reset on to the desired commit.

This PR also required mrc-ide/rrq#110 and mrc-ide/orderly2#130 for the env vars required to make this work

M-Kusumgar · 2024-03-07T16:39:53Z

R/runner.R

@@ -0,0 +1,59 @@
+runner_run <- function(orderly_root, reportname, parameters, branch, ref, ...) {
+  # Helper functions


These are here because there is a bug in rrq_task_create_call and it doesn't seem to capture them. Made a ticket about this: https://mrc-ide.myjetbrains.com/youtrack/issue/mrc-5153/Investigate-rrqtaskcreatecall-bug

Does this work if you call rrq::rrq_task_create_call with the package qualifier? i.e. as orderly.runner:::runner_run?

i tried it, didnt seem to even pick up orderly:::runner_run like the task completed successfully but it didnt do anything

ohhh nevermind, i didnt install it, it did work!

it actually works without it too, i just didnt do an install all and so it didnt pick it up from the orderly.runner package 😭 i was avoiding that cos I was relying on specific branches of the orderly2 and rrq repos and didnt know if install all would overwrite them with the head of the github repo, but yh that was the issue!

r-ash

This looks great, couple of questions/comments. I wonder if your helper functions will be available if you call your rrq submit with the package qualified name.

Is the plan to expose this in the API the next PR?

r-ash · 2024-03-08T09:16:11Z

tests/testthat/test-queue.R

+  q <- new_queue_quietly(root)
+  worker_manager <- start_queue_workers_quietly(1, q$controller)
+  make_worker_dirs(root, worker_manager$id)


Could wrap this in a helper to get a queue with specified number of workers.

r-ash · 2024-03-08T09:17:45Z

tests/testthat/helper-orderly-runner.R

+  for (i in seq_len(n_tries)) {
+    is_completed <- rrq::rrq_task_status(
+      task_id, controller = controller
+    ) == "COMPLETE"
+    if (is_completed == TRUE) {
+      break
+    }
+    Sys.sleep(1)
+  }


You can use rrq_task_wait for this

ooo thats neat thanks

r-ash · 2024-03-08T09:21:15Z

R/runner.R

@@ -0,0 +1,59 @@
+runner_run <- function(orderly_root, reportname, parameters, branch, ref, ...) {
+  # Helper functions


Does this work if you call rrq::rrq_task_create_call with the package qualifier? i.e. as orderly.runner:::runner_run?

r-ash · 2024-03-08T09:31:11Z

R/runner.R

+  # Cleanup
+  git_clean(worker_path)


In what case do we need to cleanup?

Just tried locally and I was able to use point_head_to_ref to switch between commits without cleaning up anything in between.

Is this for if orderly fails to run and writes to the draft dir? I think that will be gitignored?

at the end of a successful run, orderly deletes the file from the draft folder and leaves an empty folder in the directory, the most important bit of this git clean function is the removing of those empty dirs, just to keep a clean worktree after every run, otherwise that draft folder would just keep accumulating lots of empty folders after many runs

the stash and drop is just a safety in case someone has somehow produced something that is untracked by git, we can guarantee that it is removed and doesnt accumulate, people could also change their gitignore files which could also lead to these problems i think

I wonder if it might also be worth (or worth doing in addition) the clean before running the report?

codecov · 2024-03-08T13:49:14Z

Codecov Report

Attention: Patch coverage is 97.29730% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 99.44%. Comparing base (0c8dfbd) to head (b69faac).

❗ Current head b69faac differs from pull request most recent head 7f8f52f. Consider uploading reports for the commit 7f8f52f to get more accurate results

Files	Patch %	Lines
R/runner.R	96.42%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##              main       #5      +/-   ##
===========================================
- Coverage   100.00%   99.44%   -0.56%     
===========================================
  Files            7        8       +1     
  Lines          142      179      +37     
===========================================
+ Hits           142      178      +36     
- Misses           0        1       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

M-Kusumgar · 2024-03-08T13:49:53Z

Is the plan to expose this in the API the next PR?

yh i wanted to get this object sorted, once the status and submit are in, adding the endpoints should hopefully be a fairly easy PR

richfitz

I didn't get a chance to run this up, sorry. But the implementation looks great, much simpler than orderly1 I think. I'm sure when we use it in practice we'll iterate a few times getting the git stuff right, but this feels at least on the right not track, possibly it'll be all we need

richfitz · 2024-03-08T14:41:26Z

tests/testthat/examples/git-clean/git-clean.R

+orderly2::orderly_artefact("Some data", "data.rds")
+d <- data.frame(a = 1:10, x = runif(10), y = 1:10 + runif(10))
+write.table("test", file = file.path("..", "..", "inside_draft.txt"))
+write.table("test", file = file.path("..", "..", "..", "outside_draft.txt"))


richfitz · 2024-03-08T17:14:21Z

R/runner.R

+  # Cleanup
+  git_clean(worker_path)


I wonder if it might also be worth (or worth doing in addition) the clean before running the report?

…lt branch

r-ash · 2024-05-03T17:01:27Z

Pushed a commit here which will

Update to use latest rrq (mostly dropping the 2 from the end of a couple of functions)
Make the tests work with any configured default git branch, these were failing for me as I have my config set to initialise as "main" by default

M-Kusumgar added 15 commits February 28, 2024 16:24

update lintr, add packages and add redis script

9a98d7e

add queue R6 class

c8cb377

tests

1e1a990

documentation to finalizer

e3417ae

add docs for Queue

255f58e

refactor object so only has view on queue and does not manage workers

fd2e375

add redis to workflow

e767d0c

actually start redis

0699a33

update docs

62a98e6

fix cleanup

cb5de4a

add runner and submit function to queue

d696102

change env var names to match other PRs

855877d

got worker running with queue

3a6de8b

queue tests done

4979cd5

remove debug

6b34b85

M-Kusumgar changed the title ~~Mrc 5114~~ Mrc 5114 Add a submit function to the queue object Mar 7, 2024

M-Kusumgar and others added 2 commits March 7, 2024 16:34

Merge branch 'main' into mrc-5114

f3d0ecd

clean up diff

b1a83c9

M-Kusumgar commented Mar 7, 2024

View reviewed changes

M-Kusumgar added 2 commits March 7, 2024 17:11

fix tests by using git helpers

ff7a8a3

fix test

0fab722

M-Kusumgar requested review from richfitz and r-ash March 7, 2024 17:42

r-ash requested changes Mar 8, 2024

View reviewed changes

rob's comments

f48f338

M-Kusumgar and others added 3 commits March 8, 2024 14:12

cov, git clean test

74d2ac0

Merge branch 'main' into mrc-5114

cb4a5e5

fix tests referencing orderly.R

4bc17a2

M-Kusumgar and others added 10 commits March 8, 2024 14:28

fix helper function

7e09a01

try to fix test

70533e1

tmate

441f844

add user name and email to worker git config

6264648

remove tmate

b1daffd

test

c7d03e6

error handling

17a32aa

refactor git stash for better error handling

f4655eb

add repo arg

8c88f39

revert to tryCatch for now

53db6cb

richfitz approved these changes Mar 8, 2024

View reviewed changes

boring_grebe and others added 4 commits March 8, 2024 18:25

not running redis on mac

070805a

better if

11b747b

change identity to me

b69faac

Update to work with latest rrq, make branch tests work with any defau…

7f8f52f

…lt branch

cleanup before report run

ca6ec00

M-Kusumgar merged commit e15473b into main Aug 1, 2024
7 checks passed

M-Kusumgar mentioned this pull request Aug 2, 2024

Mrc 5585 submit endpoint #6

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mrc 5114 Add a submit function to the queue object #5

Mrc 5114 Add a submit function to the queue object #5

M-Kusumgar commented Mar 7, 2024 •

edited

Loading

M-Kusumgar Mar 7, 2024

r-ash Mar 8, 2024

M-Kusumgar Mar 8, 2024

M-Kusumgar Mar 8, 2024

M-Kusumgar Mar 8, 2024

r-ash left a comment •

edited

Loading

r-ash Mar 8, 2024

M-Kusumgar Mar 8, 2024

r-ash Mar 8, 2024

M-Kusumgar Mar 8, 2024

r-ash Mar 8, 2024

r-ash Mar 8, 2024

M-Kusumgar Mar 8, 2024

richfitz Mar 8, 2024

codecov bot commented Mar 8, 2024 •

edited

Loading

M-Kusumgar commented Mar 8, 2024

richfitz left a comment

richfitz Mar 8, 2024

richfitz Mar 8, 2024

r-ash commented May 3, 2024 •

edited

Loading

		@@ -0,0 +1,59 @@
		runner_run <- function(orderly_root, reportname, parameters, branch, ref, ...) {
		# Helper functions

Mrc 5114 Add a submit function to the queue object #5

Mrc 5114 Add a submit function to the queue object #5

Conversation

M-Kusumgar commented Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r-ash left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 8, 2024 • edited Loading

Codecov Report

M-Kusumgar commented Mar 8, 2024

richfitz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r-ash commented May 3, 2024 • edited Loading

M-Kusumgar commented Mar 7, 2024 •

edited

Loading

r-ash left a comment •

edited

Loading

codecov bot commented Mar 8, 2024 •

edited

Loading

r-ash commented May 3, 2024 •

edited

Loading