insightsengineering / tlg-catalog

A catalog of Tables, Listings and Graphs (TLGs) created with NEST R packages
https://insightsengineering.github.io/tlg-catalog/
Other
20 stars 8 forks source link

force render all articles #187

Closed pawelru closed 9 months ago

pawelru commented 10 months ago

When reading the logs from deployment I noticed that only part of the articles are being rendered (and the other part is taken from cache?) Few examples from the latest build:

We do want to render (cover) all the articles so as to test sufficiently against changes in the packages used. This PR aims to force render all. I hope this is enough.
This is desirable for both stable and devel profile so changes in the main config file.

Relevant docs: https://quarto.org/docs/projects/code-execution.html#freeze https://quarto.org/docs/projects/code-execution.html#cache

github-actions[bot] commented 10 months ago

Unit Tests Summary

  1 files  2 suites   1m 0s :stopwatch:  24 tests 0 :white_check_mark:  24 :zzz: 0 :x: 289 runs  0 :white_check_mark: 289 :zzz: 0 :x:

Results for commit 26077d0d.

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

github-actions[bot] commented 10 months ago

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
graph-snaps 💔 $7.93$ $+2.01$ $0$ $0$ $0$ $0$
markdown-snaps 💔 $49.83$ $+1.06$ $0$ $0$ $0$ $0$
Additional test case details | Test Suite | $Status$ | Time on `main` | $±Time$ | Test Case | |:-----|:----:|:----:|:----:|:-----| | graph-snaps | 💔 | $3.55$ | $+1.36$ | plot_v1 |

Results for commit 656c7e9b40020f3b28f7765c298fcb191a20d625

♻️ This comment has been updated with latest results.

pawelru commented 10 months ago

This could be a very expensive operation every time.

Yes, we should expect increased timings on each workflow run but I guess this is unavoidable if we want to test unchanged code against environment (packages) for potential incompatibility - we have to run everything. I have only one idea to address it but this probably requires more thinking and development. We can differentiate scheduled and push-triggered runs. Only the latter would use cache and the former will render everything. LMKWYT

I'd recommend just clearing the runner caches if we want to force render.

Yes that's essentially it. Would you mind push necessary changes?

cicdguy commented 10 months ago

Looks like GitHub has provided an out-of-the-box solution for cache eviction: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#force-deleting-cache-entries

I'll add it here.

cicdguy commented 10 months ago

FWIW, these caches are big, and literally expensive :)

Screenshot 2024-01-04 at 8 15 35 AM
pawelru commented 9 months ago

Hey @cicdguy I noticed that caches are gone. Is there anything required from the perspective of action definition? Things like remove cache creation / push? Would you mind review this PR if that's enough?