Add createMosaic mutation and related functionality - #4012
Conversation
nellh
left a comment
nellh
left a comment
There was a problem hiding this comment.
A few comments. This looks pretty good, if we're keeping the mosaics in the datasets volume we'd just need an endpoint to serve them (on_get for the mosaic handler would work) and a resolver for that. The advantage of moving them into a branch is we could reuse the git object endpoint to stream them but that might not be worth the added complexity.
Let's start with an admin tab button to trigger mosaic creation and automate if that seems to be working well?
| lock = client.lock(f'mosaic-lock:{dataset_id}:{ref}', timeout=60 * 60 *4) | ||
| if await lock.acquire(blocking=False): | ||
| try: | ||
| out_dir_path = Path(f"/datalad/mosaics/{dataset_id}") |
There was a problem hiding this comment.
This should use DATALAD_DATASET_PATH as this volume can be configured to be in different locations.
|
|
||
|
|
||
| @broker.task | ||
| async def create_mosaic(dataset_id, dataset_path, ref, cookies=None, user=''): |
There was a problem hiding this comment.
Looks like the ref isn't actually referenced for the mosaic being produced. It would be good to read from that version of the dataset.
There was a problem hiding this comment.
I was looking at validator.py for ideas on how to do this but it looks like ref doesn't get used there either. I can't tell if I'm missing something or if that's a bug.
| export const mosaic = async (dataset, _, _context: GraphQLContext) => { | ||
| const cache = new CacheItem( | ||
| getRedis(), | ||
| CacheType.mosaic, | ||
| [dataset.id, dataset.revision], | ||
| ) |
There was a problem hiding this comment.
The Redis cache here is pretty unnecessary for a single MongoDB query. I'd remove it to simplify this.
2 participants
This sets up the basics for incorporating the bids-mosaic tool into openneuro:
I haven't added any tests yet but happy to do so once everything else is ready to go.
I thought I would start with the basics here and then we can discuss whether we want to incorporate it within the UI and/or have it be auto-triggered. Another idea from @effigies was to keep the 2D images annexed in a separate branch so unchanged images don't have to be regenerated when running on a future snapshot (currently they get created in /tmp and then deleted).