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

When requesting data from fallback layer, skip version field #4644

Merged
merged 2 commits into from
Jun 3, 2020

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Jun 3, 2020

webknkossos-connect is strict about request formats and fails on unexpected fields. When requesting fallback layer data for volume tracings, the version field was sent to the datastore, but datastores don’t know what to do with this field. This PR removes it in this case.
This fixes scalableminds/webknossos-connect#90 (except for the still-wrong largest segment ID, but that is tracked in separate issue scalableminds/webknossos-connect#92)

The scala tracingstores and datastores ignore superfluous fields rather than fail. Do you think we should adapt this behavior for wkconnect also? @philippotto @jstriebel

Steps to test:

  • start a volume tracing with fallback layer on neuroglancer dataset (e.g. harris2015 which is preconfigured in webknossos-connect). should work as expected (except for segment ID overflow)

Issues:


@fm3 fm3 self-assigned this Jun 3, 2020
@fm3 fm3 added the frontend label Jun 3, 2020
@fm3 fm3 requested a review from philippotto June 3, 2020 10:20
@fm3 fm3 changed the title [WIP] When requesting data from fallback layer, skip version field When requesting data from fallback layer, skip version field Jun 3, 2020
@fm3 fm3 marked this pull request as ready for review June 3, 2020 10:20
@philippotto
Copy link
Member

The scala tracingstores and datastores ignore superfluous fields rather than fail. Do you think we should adapt this behavior for wkconnect also? @philippotto @jstriebel

Yes, I think this makes sense. Either way, it should be consistent. So, either both implementations should ignore superfluous fields or be strict about it. I'd do the change which is less effort and less likely to break things (so, presumably ignoring superfluous fields?).

Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you for debugging this 👍

@philippotto
Copy link
Member

fixes scalableminds/webknossos-connect#90

Cool! Didn't know the closing works across repositories :)

@fm3 fm3 merged commit ce66f20 into master Jun 3, 2020
@fm3 fm3 deleted the fallback-layer-skip-version branch June 3, 2020 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating volume annotation from existing Neuroglancer segmentation layer fails
2 participants