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

Fix assignment of sensitive git-annex metadata data via glob patterns (regression introduced by #739) #793

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

bpinsard
Copy link
Contributor

I introduced a regression in #739, and it was not flagged by the tests.

This PR fixes the glob pattern to properly flag the files as sensitive.
This is critical as if any data management pipeline rely on heudiconv setting this metadata, then sensitive data might not be handled appropriately/exposed.
I added that feature to the regression test, as this already require datalad to download data.
@yarikoptic let me know if you prefer this test to be it in a separate function.

Copy link

codecov bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.44%. Comparing base (7200a0b) to head (8da2b43).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #793      +/-   ##
==========================================
+ Coverage   82.38%   82.44%   +0.05%     
==========================================
  Files          42       42              
  Lines        4304     4312       +8     
==========================================
+ Hits         3546     3555       +9     
+ Misses        758      757       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yarikoptic
Copy link
Member

I introduced a regression in #739, and it was not flagged by the tests.

for my own sake -- what was there the regression, since I can't catch what was done differently besides making a loop over a list?

it is ok to complement that test indeed.

@bpinsard
Copy link
Contributor Author

bpinsard commented Oct 25, 2024

The changes in #739 meant to apply the metadata only on the newly converted files. This was done by passing the list of converted files to external.dlad.mark_sensitive and checking against the glob patterns provided, but the glob pattern gives anat folders, not the files in there. While git-annex is able to set metadata recursively, when filtering with the files none of the folder pattern were kept. So it ended up not adding metadata to these files. Globbing on the files in anat folder (adding wildcard) solves that.

@yarikoptic yarikoptic changed the title fix glob patterns for #739 induced regression Fix assignment of sensitive git-annex metadata data via glob patterns (regression introduced by #739) Oct 25, 2024
@yarikoptic yarikoptic added patch Increment the patch version when merged release Create a release when this pr is merged labels Oct 25, 2024
@yarikoptic
Copy link
Member

gy - #793 to fix #739 ;-) Thanks!

@yarikoptic yarikoptic merged commit 93781c7 into nipy:master Oct 25, 2024
9 checks passed
Copy link

🚀 PR was released in v1.3.1 🚀

@bpinsard
Copy link
Contributor Author

symmetrical PR ;) Thanks!
I might be the main user of the datalad sensitive marking.

@yarikoptic
Copy link
Member

nope -- just today I had to reset it for defaced images we pushed for https://github.com/spatialtopology/ds005256/ which we are yet to finalize into publishable state on openneuro ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged release Create a release when this pr is merged released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants