teal 479 mirai lockfile@main

@pawelru @m7pr This is a mirai alternative. Package seems to address the biggest issues reported during a research:

mirai has a native support of ExtendedTask
mirai is not being killed when runApp is executed (like callr does)
mirai by default opens a deamon in parallel R session without a need to handle the future::plan.
mirai has only one dependency in the whole dependency tree.

Disadvantages so far:

~~we need to pass and set options, system vars, working directory and .libPaths https://github.com/shikokuchuo/mirai/issues/122~~

How does it work:

lockfile creation is invoked in init before application starts. This prevents to start the process each time when a new shiny session starts. Process is invoked as a promise and eventually teal_app.lock will be created
When shiny session starts download lockfile button is hidden by default. If promise is eventually resolved and lockfile is created then download button is shown.
alternatively, app developer can pre-compute lockfile and provide its path in teal.renv.lockfile option. In such case renv::snapshot will be skipped and user lockfile will be used in an app.

Logs and notifications

Logs are printed for app developer while notifications are presented to the app user:

When app uses precomputed file:

log in init: Lockfile set using option "teal.renv.lockfile" - skipping automatic creation.
no notification to the app user.

When app automatically determines snapshot:

log in init: Lockfile creation started based on { getwd() }.
log If lockfile created: Lockfile {path} containing { n-pkgs } packages created{ with errors or warnings }.
notification if lockfile created: Lockfile available to download
log if lockfile not created: Lockfile creation failed.
notification if lockfile not created: Lockfile creation failed.

Jul 12 '24 12:07 gogonzo

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  ----------------------------------------------------------------------------------------------------------------------------------------
R/checkmate.R                        24       0  100.00%
R/dummy_functions.R                  49      11  77.55%   30, 32, 44, 55-62
R/get_rcode_utils.R                  12       0  100.00%
R/include_css_js.R                   22      17  22.73%   12-38, 76-82
R/init.R                            109      51  53.21%   107-114, 160-169, 171, 183-204, 229-232, 239-246, 249-250, 252
R/landing_popup_module.R             25      25  0.00%    61-87
R/module_bookmark_manager.R         158     127  19.62%   47-68, 88-138, 143-144, 156, 203, 238-315
R/module_data_summary.R             193      68  64.77%   24-52, 94, 192, 232-272
R/module_filter_data.R               53       2  96.23%   22-23
R/module_filter_manager.R           230      57  75.22%   56-62, 73-82, 90-95, 108-112, 117-118, 291-314, 340, 367, 379, 386-387
R/module_init_data.R                107      11  89.72%   44-53, 67
R/module_nested_tabs.R              167      70  58.08%   33-121, 150, 200, 258, 297
R/module_snapshot_manager.R         216     146  32.41%   89-95, 104-113, 121-133, 152-153, 170-180, 184-199, 201-208, 215-230, 234-238, 240-246, 249-262, 265-273, 304-318, 321-332, 335-341, 355
R/module_teal_data.R                114      21  81.58%   42-48, 87-90, 114-123
R/module_teal_lockfile.R            131      44  66.41%   32-36, 44-56, 59-61, 75, 85-87, 99-101, 109-118, 121, 123, 125-126, 160-161
R/module_teal_with_splash.R          12      12  0.00%    22-38
R/module_teal.R                     141      92  34.75%   42-145, 182-183, 191
R/module_transform_data.R            56      32  42.86%   17-51
R/modules.R                         181      32  82.32%   166-170, 225-228, 326-327, 359-373, 411, 423-431
R/reporter_previewer_module.R        19       2  89.47%   30, 34
R/show_rcode_modal.R                 24      24  0.00%    17-42
R/tdata.R                            14      14  0.00%    19-61
R/teal_data_module-eval_code.R       24       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                 43       0  100.00%
R/teal_data_utils.R                  39       0  100.00%
R/teal_reporter.R                    68       6  91.18%   69, 77, 125-126, 129, 146
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      63       0  100.00%
R/TealAppDriver.R                   353     353  0.00%    52-735
R/utils.R                           198       1  99.49%   344
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   110-377
R/zzz.R                              15      11  26.67%   4-18
TOTAL                              2986    1266  57.60%

Diff against main

Filename                    Stmts    Miss  Cover
------------------------  -------  ------  -------
R/init.R                       -1       0  -0.43%
R/module_teal_lockfile.R     +131     +44  +66.41%
R/utils.R                       0      +1  -0.51%
R/zzz.R                        +3      +3  -6.67%
TOTAL                        +133     +48  +0.23%

Results for commit: 3ffc3de1b4f774c28fdc6edc2f0488014e4eaa37

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

Jul 15 '24 07:07 github-actions[bot]

Unit Tests Summary

1 files 25 suites 8m 24s :stopwatch: 254 tests 248 :white_check_mark: 6 :zzz: 0 :x: 520 runs 514 :white_check_mark: 6 :zzz: 0 :x:

Results for commit 3ffc3de1.

:recycle: This comment has been updated with latest results.

Jul 15 '24 07:07 github-actions[bot]

Unit Test Performance Difference

Test Suite	$Status$	Time on `main`	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
module_teal	💔	$41.92$	$+14.89$	$+3$	$0$	$0$	$0$
shinytest2-data_summary	💚	$38.75$	$-2.71$	$0$	$0$	$0$	$0$
shinytest2-filter_panel	💚	$43.60$	$-2.37$	$0$	$0$	$0$	$0$
shinytest2-landing_popup	💚	$46.13$	$-3.43$	$0$	$0$	$0$	$0$
shinytest2-module_bookmark_manager	💚	$35.67$	$-1.24$	$0$	$0$	$0$	$0$
shinytest2-modules	💚	$39.84$	$-2.13$	$0$	$0$	$0$	$0$
shinytest2-wunder_bar	💚	$22.75$	$-1.63$	$0$	$0$	$0$	$0$
utils	💚	$67.39$	$-67.24$	$-8$	$0$	$0$	$0$

Additional test case details

Test Suite	$Status$	Time on `main`	$±Time$	Test Case
module_teal	👶		$+17.41$	creation_process_is_invoked_for_teal.lockfile.mode_enabled_and_snapshot_is_copied_to_teal_app.lock_and_removed_after_session_ended
module_teal	👶		$+0.05$	creation_process_is_not_invoked_for_teal.lockfile.mode_disabled_
shinytest2-data_summary	💚	$15.26$	$-1.70$	e2e_data_summary_is_displayed_properly_if_teal_data_include_data.frames_with_join_keys_MAE_objects_and_vectors
shinytest2-filter_panel	💚	$11.05$	$-1.03$	e2e_module_content_is_updated_when_a_data_is_filtered_in_filter_panel
shinytest2-landing_popup	💚	$9.64$	$-1.54$	e2e_app_with_customized_landing_popup_module_creates_modal_containing_specified_title_content_and_buttons
shinytest2-modules	💚	$9.77$	$-1.45$	e2e_all_the_nested_teal_modules_are_initiated_as_expected
utils	💀	$0.02$	$-0.02$	check_modules_datanames_message_is_the_same_in_html_tags_and_in_string
utils	💀	$67.21$	$-67.21$	create_renv_lockfile_creates_a_lock_file_during_the_execution

Results for commit 481971cc019884886b61f3616e74a0ee316af92c

♻️ This comment has been updated with latest results.

Jul 15 '24 07:07 github-actions[bot]

Overall looks good based on findings researched during future+shiny::ExtendedTask and callr approaches :)

Jul 15 '24 11:07 m7pr

There is also a NOTE in R CMD CHECK that we need to eliminate

* checking R code for possible problems ... NOTE
teal_lockfile_invoke : <anonymous>: no visible global function
  definition for ‘run’
teal_lockfile_invoke : <anonymous>: no visible binding for global
  variable ‘opts’
teal_lockfile_invoke : <anonymous>: no visible binding for global
  variable ‘sysenv’
teal_lockfile_invoke : <anonymous>: no visible binding for global
  variable ‘libpaths’
teal_lockfile_invoke : <anonymous>: no visible binding for global
  variable ‘wd’
Undefined global functions or variables:
  libpaths opts run sysenv wd

Jul 16 '24 10:07 m7pr

I think the function in process creation needs to be defined as a regular function

  process <- ExtendedTask$new(function(...) {
    mirai::mirai(
      run(lockfile_path = lockfile_path, opts = opts, sysenv = sysenv, libpaths = libpaths, wd = wd),
      ...
    )
  })

  process <- ExtendedTask$new(run_mirai)
  run_mirai <- function(lockfile_path, opts, sysenv, libpaths, wd) {
  mirai::mirai(
      renv_snapshot(lockfile_path = lockfile_path, opts = opts, sysenv = sysenv, libpaths = libpaths, wd = wd),
    )
 }

Jul 16 '24 10:07 m7pr

run(lockfile_pa

you probably need to teal:::renv_lockfile then.

Jul 16 '24 11:07 gogonzo

I see some errors on vignettes creations during R CMD CHECK https://github.com/insightsengineering/teal/actions/runs/9956572489/job/27506766811#step:23:35

Quitting from lines 106-112 [unnamed-chunk-4] (adding-support-for-reporting.Rmd)
Error: processing vignette 'adding-support-for-reporting.Rmd' failed with diagnostics:
Don't know how to convert object of class NULL into a promise
--- failed re-building ‘adding-support-for-reporting.Rmd’

--- re-building ‘bootstrap-themes-in-teal.Rmd’ using rmarkdown
--- finished re-building ‘bootstrap-themes-in-teal.Rmd’

--- re-building ‘creating-custom-modules.Rmd’ using rmarkdown

Quitting from lines 174-183 [unnamed-chunk-4] (creating-custom-modules.Rmd)
Error: processing vignette 'creating-custom-modules.Rmd' failed with diagnostics:
Don't know how to convert object of class NULL into a promise
--- failed re-building ‘creating-custom-modules.Rmd’

--- re-building ‘data-as-shiny-module.Rmd’ using rmarkdown

Quitting from lines 40-68 [unnamed-chunk-2] (data-as-shiny-module.Rmd)
Error: processing vignette 'data-as-shiny-module.Rmd' failed with diagnostics:
Don't know how to convert object of class NULL into a promise
--- failed re-building ‘data-as-shiny-module.Rmd’

--- re-building ‘filter-panel.Rmd’ using rmarkdown
--- finished re-building ‘filter-panel.Rmd’

--- re-building ‘getting-started-with-teal.Rmd’ using rmarkdown

Quitting from lines [33](https://github.com/insightsengineering/teal/actions/runs/9956572489/job/27506766811#step:23:34)-52 [unnamed-chunk-1] (getting-started-with-teal.Rmd)
Error: processing vignette 'getting-started-with-teal.Rmd' failed with diagnostics:
Don't know how to convert object of class NULL into a promise
--- failed re-building ‘getting-started-with-teal.Rmd’

Jul 17 '24 08:07 m7pr

I opened a PR that address most of my comments: https://github.com/insightsengineering/teal/pull/1272

Jul 23 '24 15:07 pawelru

After discussion with @ruckip we decide to:

encapsulate everything in ui_renv_lockfile and srv_renv_lockfile
move envocation of the lockfile process to the srv_renv_lockfile which should be conditional - if file already exist, don't start the process again. It means it will no longer be in teal::init, so we don't need to pass lockfile_process to the srv_teal
we need an option(teal.renv.enable = TRUE/FALSE) to enable/disable lockfile calculation

Jul 30 '24 09:07 gogonzo

move envocation of the lockfile process to the srv_renv_lockfile which should be conditional - if file already exist, don't start the process again. It means it will no longer be in teal::init, so we don't need to pass lockfile_process to the srv_teal

It means it will no longer be in teal::init, so we don't need to pass lockfile_process to the srv_teal

I thought we wanted to have lockfile creation in init so it is created once per app, and not per each session. Is it still relevant?

Jul 31 '24 07:07 m7pr

I've found a serious problem when invoked ExtendedTask in tests. There is a long queue of running processes which are being cumulated by each srv_teal call. I'm trying to find a way to terminate on shiny::onStop.