Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

Fix issue where dependencies were not being tracked when checking produced files #8

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

r-ash
Copy link
Contributor

@r-ash r-ash commented Apr 4, 2023

If we have a setup like

/tmp/RtmpQ8SMJI/file3f9221513272c
├── archive
│   └── a
│       └── 20230404-162405-5caf9730
│           ├── data.rds
│           ├── orderly.yml
│           └── script.R
├── draft
│   └── a
├── orderly_config.yml
└── src
    ├── a
    │   ├── orderly.yml
    │   └── script.R
    └── b
        ├── orderly.yml
        └── script.R

where a just saves out an rds as data.rds then b has yml

script: script.R
artefacts:
  data:
    description: Random data
    filenames: data.rds
depends:
  - a:
      id: latest
      use:
        input.rds: data.rds

And the script

x <- readRDS("input.rds")
saveRDS(x + runif(10), "data.rds")

When running we get a message "Some extra files found: 'input.rds'"
This is because when running

  1. orderly copies dependency from A into working dir as input.rds
  2. Runs script saving out data.rds
  3. Checks that files on disk in working dir are either from src directory or are artefacts.

In particular 3. doesn't check for dependencies and so raises the "extra files" message. This doesn't happen for existing dependency test as that just copies the file over so that file is both the dependency and the artefact.

This PR will

  • Add test setup (which I want to put in place for testing pulling in dependencies with usedby and uses queries
  • Fix the issue mentioned above

There were a couple of ways we could have got the expected dependency info out, we also have it available in custom_metadata or could have read it out when reading the yml. Not sure what is most appropriate here so let me know if you want a bit of a different approach.

@r-ash r-ash requested a review from richfitz April 4, 2023 16:59
@codecov
Copy link

codecov bot commented Apr 4, 2023

Codecov Report

Merging #8 (a61dabc) into main (ccb8d25) will not change coverage.
The diff coverage is 100.00%.

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

@@            Coverage Diff            @@
##              main        #8   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          417       420    +3     
=========================================
+ Hits           417       420    +3     
Impacted Files Coverage Δ
R/orderly.R 100.00% <100.00%> (ø)

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant