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

Error when trying to submit plugin to chrisstore.co #291

Open
grdryn opened this issue Sep 18, 2022 · 4 comments
Open

Error when trying to submit plugin to chrisstore.co #291

grdryn opened this issue Sep 18, 2022 · 4 comments

Comments

@grdryn
Copy link

grdryn commented Sep 18, 2022

Hi!

I've just tried submitting my first plugin through the chrisstore.co UI, and it's giving me an error that something is not a valid string.

Here's the POST Data of the request:

-----------------------------727047094977636124258969607
Content-Disposition: form-data; name="name"

pl-heudiconv
-----------------------------727047094977636124258969607
Content-Disposition: form-data; name="dock_image"

quay.io/rh-impact/pl-heudiconv:0.1
-----------------------------727047094977636124258969607
Content-Disposition: form-data; name="public_repo"

https://github.com/rh-impact/pl-heudiconv
-----------------------------727047094977636124258969607
Content-Disposition: form-data; name="descriptor_file"; filename="blob"
Content-Type: application/json

{"type":"ds","parameters":[{"name":"inputdir_type","type":"str","optional":true,"flag":"--inputdir-type","short_flag":"--inputdir-type","action":"store","help":"For input, heudiconv accepts EITHER a \"--files\"\n            argument, or a \"--dicom_dir_template\" argument. This\n            option specifies which to use for the positional\n            \"inputdir\" argument to this plugin.","default":"files","ui_exposed":true},{"name":"heuristic","type":"str","optional":true,"flag":"--heuristic","short_flag":"-f","action":"store","help":"Name of a known heuristic or path to the Python\n            script containing heuristic.","default":"reproin","ui_exposed":true},{"name":"bids","type":"bool","optional":true,"flag":"--bids","short_flag":"-b","action":"store_true","help":"Flag for output into BIDS structure. Can also take\n            BIDS-specific options by using --bids-options, e.g.,\n            --bids-options notop.","default":false,"ui_exposed":true},{"name":"bids_options","type":"str","optional":true,"flag":"--bids-options","short_flag":"--bids-options","action":"store","help":"The only currently supported bids options is\n             \"notop\", which skips creation of top-level BIDS\n             files. This is useful when running in batch mode to\n             prevent possible race conditions.","default":[],"ui_exposed":true},{"name":"overwrite","type":"bool","optional":true,"flag":"--overwrite","short_flag":"--overwrite","action":"store_true","help":"Overwrite existing converted files.","default":false,"ui_exposed":true},{"name":"datalad","type":"bool","optional":true,"flag":"--datalad","short_flag":"--datalad","action":"store_true","help":"Store the entire collection as DataLad\n            dataset(s). Small files will be committed directly to git,\n            while large to annex. New version (6) of annex\n            repositories will be used in a \"thin\" mode so it would\n            look to mortals as just any other regular directory\n            (i.e. no symlinks to under .git/annex). For now just for\n            BIDS mode.","default":false,"ui_exposed":true},{"name":"minmeta","type":"bool","optional":true,"flag":"--minmeta","short_flag":"--minmeta","action":"store_true","help":"Exclude dcmstack meta information in sidecar jsons.","default":false,"ui_exposed":true},{"name":"random_seed","type":"int","optional":true,"flag":"--random-seed","short_flag":"--random-seed","action":"store","help":"Random seed to initialize RNG.","default":[],"ui_exposed":true},{"name":"locator","type":"str","optional":true,"flag":"--locator","short_flag":"-l","action":"store","help":"Study path under outdir. If provided, it overloads\n            the value provided by the heuristic. If --datalad is\n            enabled, every directory within locator becomes a\n            super-dataset thus establishing a hierarchy. Setting to\n            \"unknown\" will skip that dataset.","default":[],"ui_exposed":true},{"name":"session","type":"str","optional":true,"flag":"--ses","short_flag":"-ss","action":"store","help":"Session for longitudinal study_sessions. Default is\n            None.","default":[],"ui_exposed":true},{"name":"with_prov","type":"bool","optional":true,"flag":"--with-prov","short_flag":"-p","action":"store_true","help":"Store additional provenance information.","default":false,"ui_exposed":true},{"name":"command","type":"str","optional":true,"flag":"--command","short_flag":"--command","action":"store","help":"Custom action to be performed on provided files\n            instead of regular operation.","default":[],"ui_exposed":true},{"name":"anon_cmd","type":"str","optional":true,"flag":"--anon-cmd","short_flag":"--anon-cmd","action":"store","help":"Command to run to convert subject IDs used for DICOMs to anonymized IDs. Such command must take a single argument and return a single anonymized ID.","default":[],"ui_exposed":true},{"name":"subjects","type":"str","optional":true,"flag":"--subjects","short_flag":"-s","action":"store","help":"List of subjects - required for dicom template. If\n            not provided, DICOMS would first be \"sorted\" and subject\n            IDs deduced by the heuristic.","default":[],"ui_exposed":true},{"name":"grouping","type":"str","optional":true,"flag":"--grouping","short_flag":"-g","action":"store","help":"How to group dicoms (default: by studyUID)","default":[],"ui_exposed":true},{"name":"dcmconfig","type":"str","optional":true,"flag":"--dcmconfig","short_flag":"--dcmconfig","action":"store","help":"JSON file for additional dcm2niix configuration.","default":[],"ui_exposed":true}],"icon":"","authors":"grdryn <[email protected]>","title":"A ChRIS plugin for heudiconv","category":"","description":"An app to organize brain imaging data into structured directory layouts","documentation":"https://github.com/rh-impact/pl-heudiconv","license":"MIT","version":"0.1","selfpath":"/usr/local/bin","selfexec":"pl-heudiconv","execshell":"/usr/local/bin/python","max_number_of_workers":1,"min_number_of_workers":1,"max_memory_limit":"","min_memory_limit":8000,"max_cpu_limit":"","min_cpu_limit":2000,"max_gpu_limit":0,"min_gpu_limit":0}
-----------------------------727047094977636124258969607--

and here's the response:

{"collection":{"version":"1.0","href":"https://chrisstore.co/api/v1/plugins/","error":{"message":"{\"value\": [\"Not a valid string.\"]}"}}}
  1. Where am I going wrong?
  2. Would this error message be more useful if it was more specific?

Thanks!

@grdryn
Copy link
Author

grdryn commented Oct 10, 2022

I've gotten the store running locally now so that I can test it, and I believe I've narrowed down the problem to invalid defaults that I've set for some typed arguments in the argparse in the plugin. E.g. [] as the default for string or int args rather than, say '' or 0 respectively. The argparse module in the plugin app seems fine with those, but the validation on the JSON produced by --savejson that happens on submission to the store doesn't like it.

@jennydaman
Copy link
Collaborator

Ah, sounds like some Python weak-typing fun. This could be a bug in https://github.com/fnndsc/chrisapp, though we have some newer, more sparkly machinery, https://github.com/fnndsc/chris_plugin

It still stands that the ChRIS_store_ui (and perhaps the ChRIS_store backend) could be more helpful here in displaying validation error messages. Thanks for digging around @grdryn

@grdryn
Copy link
Author

grdryn commented Oct 13, 2022

This could be a bug in https://github.com/fnndsc/chrisapp, though we have some newer, more sparkly machinery, https://github.com/fnndsc/chris_plugin

Thanks @jennydaman, I have a TODO item () to convert my plugin to the new structure of https://github.com/FNNDSC/python-chrisapp-template. Can you tell me how python-chrisapp-template is related to the chris_plugin project that you mention? Should I be looking instead at chris_plugin, or is it a separate concern?

It still stands that the ChRIS_store_ui (and perhaps the ChRIS_store backend) could be more helpful here in displaying validation error messages.

Yes, a list of error messages in the response might be a good approach: one for each validation failure 🤔

@jennydaman
Copy link
Collaborator

@grdryn chris_plugin is a Python module, python-chrisapp-template is starter code which uses the chris_plugin module. I recommend you look at python-chrisapp-template (which might lead you to reading the documentation on chris_plugin)

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

No branches or pull requests

2 participants