Relocate test data to bucket - #994
Conversation
There was a problem hiding this comment.
Did you intent to bundle the whole input directory together? I think that means that if one file changes, everything gets re-downloaded. Not too bad right now at 300MB, but this could get annoying. We can also do it per file. Or perhaps per model, but then we should probably bundle the models in subdirectories.
There was a problem hiding this comment.
Right now we bundle all files (version): https://github.com/visr/wflow-artifacts/releases/tag/v1.0.0.
There was a problem hiding this comment.
Yes I think it is fine like this. If it starts to get annoying because people are changing different files in different branches, we can still decide to version them separately.
And it doesn't seem to be true that everything gets redownloaded.
There was a problem hiding this comment.
Now I see why I was wrong. DVC doesn't track the dir as one blob, it just stores a JSON with pointers to the actual files, see this eac653f3009e1f9babcae88c07ee56.dir for instance:
[
{
"md5":"76afca075bef6e20f48eb11dd585d7a4",
"relpath":"hydrotype.dbf"
},
{
"md5":"cb3e648c38fd75188df5ea80c1924386",
"relpath":"hydrotype.prj"
},
{
"md5":"b5ed91a25adf2925fc573061e948d6ff",
"relpath":"hydrotype.shp"
},
{
"md5":"2806f374363203bf1e88ce3b3ee238f2",
"relpath":"hydrotype.shx"
},
{
"md5":"399c079845032dfa1749335c619725e3",
"relpath":"vdGaast_water_depth.csv"
}
]| using Downloads | ||
|
|
||
| const source_url = "https://github.com/visr/wflow-artifacts/releases/download" | ||
| const version = v"1.0.0" |
There was a problem hiding this comment.
What about providing the version of the test data (now equal to Wflow version (major and minor))?
There was a problem hiding this comment.
Not sure exactly what you mean. In a way it is now versioned itself, with a hash tracked in git.
|
Was it considered that anonymous access should be possible? If I understand correctly, as it is right now people can only get the test data if they have access keys. That means that external people cannot get the data unless they have such a key, and since secrets are or shared with PRs from forks, CI will also not run from forks. That is different from the current situation, where anybody can get the test data without authentication. We should be able to support anonymous read access with |
visr
left a comment
There was a problem hiding this comment.
I discussed the anonymous access (#994 (comment)) with @JoostBuitink, and we think it is important to address this within this PR as well. That probably should've come up earlier, sorry for adding that requirement now.
Issue addressed
Fixes #723
Fixes #376
Explanation
Explain how you addressed the bug/feature request, what choices you made and why.
Checklist
masterAGENTS.mdare satisfiedAdditional Notes (optional)
Add any additional notes or information that may be helpful.