-
Notifications
You must be signed in to change notification settings - Fork 0
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
Auto aux update check #94
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @shahronak47 ,
I noticed you're relying on the modified date to track changes in aux files. While that works, it could be risky if they're edited multiple times in a day.
For enhanced security and clarity, I propose using the hashes created by the pipfun::pip_sign_save()
function. This function calculates and saves a unique hash each time an aux file is updated, ensuring precise identification of different versions.
Please review the details of pipfun::pip_sign_save()
and the hash files saved. Let me know if you agree with a hash-based approach. If not, we can explore further improvements to the current date-based method.
Thanks.
R/auto_aux_update.R
Outdated
# If the files that we wanted to updated were updated today only then write the file | ||
temp_dat <- out |> | ||
dplyr::filter(filename %in% paste0(aux_fns, ".qs")) |> | ||
dplyr::filter(as.Date(time_last_update) == Sys.Date()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't Think this will work well, especially since it is possible to update the same file several times a day
Instead of today's date we can look if the file is updated in last x mins. We can define x as 5 or 10 mins. Using hash is more safer however, it can be a lot of work to get that hash for each file which is updated because |
Hi @shahronak47 , Right where the aux file is saved, there is another file called |
fix formating a little
Clickup task - https://app.clickup.com/t/868763th2
I think I have a fix, but let's test it when we have aux data to update before merging.