Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create an "always stale" target that generates the hash table of parser functions #242

Merged
merged 5 commits into from
Dec 6, 2021

Conversation

jordansread
Copy link

@jordansread jordansread commented Dec 3, 2021

This is a partial for #231 , and focuses on solving a single problematic part of the pull: there is a target that generates the hash table of parser functions, but required manual intervention to trigger an update (the prior way was to edit the dummy string). This was generating issues, because if you made an edit to a parser function or added a new one, you wouldn't necessarily send those changes on to the parsing of the files. You'd discover this when you got an error because there was a cooperator-added file that didn't have a parser ("but I added that file!") and then realize you need to go back and change the dummy argument. The worse case was when you made a bug fix to a parser and thought you propagated the changes with scmake() but the changes weren't recognized because they are effectively hidden from remake (the parser files are referenced by a directory of files, not the individual files in the recipe).

This is a scipiper-specific fix which does introduce some new codes and concepts even though I think we're doing most of the new stages and refactors with targets. But I found this issue to be too error-prone to have it keep biting us, so this could be a solution moving forward or could be replaced if the coop section of this pipeline is refactored with targets due to the stage's continued use/development and the other brittle components that still exist and are or will be documented in #231

What this solution does is force 7a_temp_coop_munge/tmp/parser_files.yml and coop_parsers to both be stale on every single build. These are quick targets to generate, so this doesn't cost much. If neither of these targets actually change after rebuild, the result of the task table isn't rebuilt too, so that works as expected:

image

Copy link
Contributor

@limnoliver limnoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good solution to this problem!

#' @details this function is used to keep a dependency stale always, assuming this function is called whenever
#' that file is referenced and there is at least a paired target use of the file.
make_file_stale <- function(filepath){
cat(file = filepath, paste0(format(Sys.time(), '%m/%d/%y %H:%M:%S; random ID: '), sample(1E6, 1)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of the random ID (as opposed to just MM-DD-YYYY H:M:S) ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I added a commit with more documentation explaining this. The random number is there because we want
to write something unique to the file, and there is a potential that this make_file_stale() function could be called several times in one second. Without the random ID added to it, the calls mad within the same second won't modify the file uniquely, and then it won't make the other targets stale.

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

Successfully merging this pull request may close these issues.

2 participants