mrc-3665 way for orderly web to control demo data #320

hillalex · 2022-10-06T11:33:00Z

This adds a new entrypoint that exposes the function for generating an orderly demo based on a source directory and a demo.yml file. See this in use in OrderlyWeb: vimc/orderly-web#479

R/testing.R is a bit of a tangle tbh, but rather than unpicking it I've just added a new function for what we want in OW.

codecov · 2022-10-06T11:50:01Z

Codecov Report

Merging #320 (efa0c81) into master (5b12ff9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

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

@@           Coverage Diff           @@
##           master     #320   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files          41       41           
  Lines        4524     4535   +11     
=======================================
+ Hits         4522     4533   +11     
  Misses          2        2

Impacted Files	Coverage Δ
R/testing.R	`98.80% <100.00%> (+0.08%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

hillalex · 2022-10-17T10:54:31Z

inst/run_orderly_demo.sh

@@ -0,0 +1,9 @@
+#!/bin/sh


I think the existing create_orderly_demo script is still in use by things other than OW (e.g. for testing in this repo), so I've left that as is and just added a new one for the usage we want in OW.

Looks like create_orderly_demo script is used in test set in many places, like montagu-proxy, montagu-reporting-api, montagu-webapps. If we are not using create_orderly_demo in orderly-web any more then possibly worth backing out commit d7922bd from this PR to avoid potentially breaking the other components which use it

Ok yep, let's do that.

r-ash

New function for running git example from source looks good. Small comment about the changes to create_orderly_demo script.

It would be nice to review all the setup functions in R/testing.R at some point as they have all expanded since first being added and I think probably have some overlap in functionality. The differences between them are quite confusing. Can see that is out of scope for what you want to do in this ticket though

r-ash · 2022-10-18T11:32:05Z

inst/run_orderly_demo.sh

@@ -0,0 +1,9 @@
+#!/bin/sh


Looks like create_orderly_demo script is used in test set in many places, like montagu-proxy, montagu-reporting-api, montagu-webapps. If we are not using create_orderly_demo in orderly-web any more then possibly worth backing out commit d7922bd from this PR to avoid potentially breaking the other components which use it

hillalex · 2022-10-18T12:07:21Z

New function for running git example from source looks good. Small comment about the changes to create_orderly_demo script.

It would be nice to review all the setup functions in R/testing.R at some point as they have all expanded since first being added and I think probably have some overlap in functionality. The differences between them are quite confusing. Can see that is out of scope for what you want to do in this ticket though

Agree that adding a further function feels like adding more complication on top of an already confusing collection of code. I'd be happy to have a go at refactoring the whole file in another ticket.

hillalex · 2022-10-18T12:44:37Z

Ok, I've backed out the previous change @r-ash had made, and this just adds the new entrypoint.

hillalex · 2022-10-20T12:06:17Z

I'm a little puzzled as to why the windows R-CMD-check is failing, given that it seems to be passing for other branches...

richfitz · 2022-10-20T12:37:01Z

quite likely it's failing because openssl has been updated on cran, GHA is trying and failing to install it from source - probably something in that vignette is using openssl indirectly

richfitz · 2022-10-20T12:37:33Z

indeed that is what's going on:

Error: Error: processing vignette 'orderly.Rmd' failed with diagnostics:
there is no package called 'openssl'
--- failed re-building 'orderly.Rmd'
--- re-building 'patterns.Rmd' using rmarkdown
--- finished re-building 'patterns.Rmd'
--- re-building 'remote.Rmd' using rmarkdown
--- finished re-building 'remote.Rmd'
SUMMARY: processing the following files failed:
  'bundles.Rmd' 'orderly.Rmd'
Error: Error: Vignette re-building failed.

richfitz

Don't worry about the windows failing build, this is affecting lots of things atm and it's very unlikely this set of changes will break that build specifically

richfitz · 2022-10-20T13:25:20Z

R/testing.R

 }


+prepare_git_example_from_source <- function(source_path, path) {


Can you add a test for this within the testthat tests please?

richfitz · 2022-10-20T13:26:45Z

inst/run_orderly_demo.sh

@@ -0,0 +1,9 @@
+#!/bin/sh


Generally I think it's nicer to avoid using extensions on scripts as then we can reimplement them in another language later without having to update uses. Your editor should pick up syntax highlighting properly based on the first line

r-ash · 2022-10-20T13:48:21Z

quite likely it's failing because openssl has been updated on cran, GHA is trying and failing to install it from source - probably something in that vignette is using openssl indirectly

There is a fix for this in #321 https://github.com/vimc/orderly/pull/321/files#diff-9c940e8ad2b7bc4c26ec3da57b94bc00e73e2166cfed689da51a4c59bcc0a310R80

hillalex · 2022-10-20T13:52:54Z

docker/Dockerfile

 RUN R CMD INSTALL /orderly && \
  Rscript -e 'orderly:::write_script("/usr/bin")' && \
  cp /orderly/inst/create_orderly_demo.sh /usr/bin/ && \
+  cp /orderly/inst/run_orderly_demo /usr/bin/ && \


I've left the old script create_orderly_demo.sh with the extension, since it could be in use by other repos

richfitz · 2022-10-20T14:18:31Z

Any idea why all GHA checks suddenly stopped running here?

Edit: these ones do look like they're running but not posting back - https://github.com/vimc/orderly/actions/runs/3290165552

hillalex · 2022-10-20T14:35:40Z

Huh, weird. Also looks like there are some strange test failures, will investigate.

hillalex changed the title ~~Mrc 3665~~ mrc-3665 way for orderly web to control demo data Oct 17, 2022

hillalex commented Oct 17, 2022

View reviewed changes

hillalex marked this pull request as ready for review October 17, 2022 10:55

hillalex requested review from r-ash and richfitz October 17, 2022 10:55

hillalex mentioned this pull request Oct 17, 2022

mrc-3665 give this repo control over the orderly demo data vimc/orderly-web#479

Merged

r-ash reviewed Oct 18, 2022

View reviewed changes

hillalex force-pushed the mrc-3665 branch from 112c567 to 3b42978 Compare October 18, 2022 12:32

add new entrypoint

36a0f28

hillalex force-pushed the mrc-3665 branch from 6258b72 to 36a0f28 Compare October 18, 2022 12:43

update version and news

65d4318

hillalex requested a review from r-ash October 18, 2022 12:58

richfitz requested changes Oct 20, 2022

View reviewed changes

remove .sh extension, add test

be9f1e4

hillalex force-pushed the mrc-3665 branch from 8f1ddc7 to be9f1e4 Compare October 20, 2022 13:52

hillalex commented Oct 20, 2022

View reviewed changes

hillalex requested a review from richfitz October 20, 2022 13:53

hillalex added 2 commits October 20, 2022 15:36

unlink path after test

2303fdb

unlink temp dir and reset options

ea8e611

richfitz approved these changes Oct 20, 2022

View reviewed changes

hillalex merged commit 7d469c5 into master Oct 26, 2022

		}


		prepare_git_example_from_source <- function(source_path, path) {

mrc-3665 way for orderly web to control demo data #320

mrc-3665 way for orderly web to control demo data #320

Uh oh!

Conversation

hillalex commented Oct 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hillalex Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

r-ash Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

hillalex Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

r-ash left a comment

Choose a reason for hiding this comment

Uh oh!

r-ash Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

hillalex commented Oct 18, 2022

Uh oh!

hillalex commented Oct 18, 2022

Uh oh!

hillalex commented Oct 20, 2022

Uh oh!

richfitz commented Oct 20, 2022

Uh oh!

richfitz commented Oct 20, 2022

Uh oh!

richfitz left a comment

Choose a reason for hiding this comment

Uh oh!

richfitz Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

richfitz Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

r-ash commented Oct 20, 2022

Uh oh!

hillalex Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

richfitz commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hillalex commented Oct 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hillalex commented Oct 6, 2022 •

edited

Loading

codecov bot commented Oct 6, 2022 •

edited

Loading

richfitz commented Oct 20, 2022 •

edited

Loading