ambrussimon commented 6 years ago

@ehlertjd @nagem

We were doing some tests for the new analysis endpoints this afternoon and I realized that inputs from SDK analyses are getting saved as a list of file references rather than a copy of the file object. Also, the API doesn’t check to ensure the inputs exist in the system

I looked into both and couldn't recreate the behavior:

Tests were already in place for checking whether filerefs are turned into fileinfos
Added new tests that show filerefs are also verified to ensure they exist in the system
My only thin guess is that the "fallback to legacy" mechanism is a bit loose and that legacy placer was triggered during SDK tests, which in turn only checked anyOf
^ Fixed by separating out legacy input schema
Also found that analysis notes input schemas were pointing to notes-list-output, fixed

Review Checklist

Tests were added to cover all code changes
Documentation was added / updated
Code and tests follow standards in CONTRIBUTING.md

ambrussimon commented 6 years ago

Please provide repro steps if the problem wasn't solved by this.

codecov-io commented 6 years ago

Codecov Report

Merging #1104 into master will increase coverage by 0.01%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1104      +/-   ##
==========================================
+ Coverage   90.91%   90.92%   +0.01%     
==========================================
  Files          49       49              
  Lines        7055     7055              
==========================================
+ Hits         6414     6415       +1     
+ Misses        641      640       -1

ehlertjd commented 6 years ago

@ambrussimon Added an integration test to test_analyses.py demonstrating the problem.

ambrussimon commented 6 years ago

@ehlertjd Perfect, thank you! Fixed.

nagem commented 6 years ago

Thanks for the quick fix @ambrussimon, LGTM

scitran / core

Analysis fix #1104

Review Checklist

Codecov Report