Skip to content

Treat .ngff.source as .ngff/.zarr in upload#1077

Closed
yarikoptic wants to merge 6 commits intomasterfrom
bf-bids-suffixes
Closed

Treat .ngff.source as .ngff/.zarr in upload#1077
yarikoptic wants to merge 6 commits intomasterfrom
bf-bids-suffixes

Conversation

@yarikoptic
Copy link
Member

Alerted to by @jwodder in #1071 (comment) . Added a helper has_suffixes.

Co-authored-by: John T. Wodder II <jwodder@users.noreply.github.com>
else:
for fileclass in LocalFileAsset.__subclasses__():
if filepath.suffix in fileclass.EXTENSIONS:
if has_suffixes(filepath.suffix, fileclass.EXTENSIONS):
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure we want, say, foo.nwb.bar to be treated as an NWB?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, good point and sounds odd... might cause all kinds of side-effects for some ad-hoc compressed files etc. May be this entire PR is not worth it since case was quite ad-hoc.

WDYT @satra -- should we treat some .zarr.source/ as a Zarr or let's forget about that?

yarikoptic and others added 3 commits July 19, 2022 12:42
Co-authored-by: John T. Wodder II <jwodder@users.noreply.github.com>
Co-authored-by: John T. Wodder II <jwodder@users.noreply.github.com>
@satra
Copy link
Member

satra commented Jul 19, 2022

those are indeed mistakes in upload the source folders shouldn't have gone out. i've alerted folks.

@yarikoptic
Copy link
Member Author

those are indeed mistakes in upload the source folders shouldn't have gone out. i've alerted folks.

well , the question is what should we do to prevent "hammer effect" of such user mistakes? because now it would have lead to upload of each and one of those files as a separate asset, thus populating 100k assets in the DB. It should survive but clearly not a pleasant thing to do if could be avoided... I think this PR we can just close, but let's continue on that question in #1079

@yarikoptic yarikoptic closed this Jul 19, 2022
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.

3 participants