-
Notifications
You must be signed in to change notification settings - Fork 29
Validate DataSources more strictly when reading from disk #8894
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
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Code looks good & testing went good as well.
I left 1 question :)
case (Full(newUsableFromDB), _) => Fox.successful(newUsableFromDB) | ||
case (_, Some(dataSourceFromDir)) => Fox.successful(dataSourceFromDir) | ||
case _ => Fox.failure("DataSource not found") ~> NOT_FOUND | ||
} |
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.
Why did you add this change? Is there a possible scenario where the datasource cannot be retrieved from the cache but exists on disk / in dir?
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.
Yes, the cache only handles usable datasources. If the existing datasource is unusable due to an error, the cache will just return a Failure. However, in this context, we want to re-read the one from disk in this case, re-validate it, and sent it to webknossos if it is now usable. Only after that, it will also be available through the cache. So this part is actually the bugfix for the reload button in the dashboard (for non-virtual datasets). Does that make sense?
full datasource validation now also runs when reading datasource-properties.jsons from disk. The result is shown in the dashboard on “show error”.
The same validation also runs in the reserveManualUpload route
This also fixes a regression from #8844 where you could no longer hit reload on unusable datasets.
It has, however, been slightly relaxed (no more path checks as those have been moved elsewhere in #8844 )
Steps to test:
Issues:
$PR_NUMBER.md
file inunreleased_changes
or use./tools/create-changelog-entry.py
)