ModelSEED / ProbModelSEED

Other
2 stars 3 forks source link

no param checking #14

Open nconrad opened 9 years ago

nconrad commented 9 years ago

It appears that runfba will still run no matter what is sent to it it? This is unfriendly and makes testing difficult.

Here I set 'minfluxsddf' to -23:

{"version":"1.1","method":"ProbModelSEED.FluxBalanceAnalysis","id":"0022221722174435854","params":[{"model":"/nconrad@patricbrc.org/home/models/testgenome1.genome_model","media_supplement":[],"thermo_const_type":"None","minfluxsddf":-23}]}

Response:

{"version":"1.1","id":"0022221722174435854","result":[["fba.23","fba","/nconrad@patricbrc.org/home/models/.testgenome1.genome_model/fba/","2015-06-04T23:08:47","A7854650-0B0E-11E5-9AEC-C3F8682E0674","nconrad@patricbrc.org",15047,{"media":"/chenry/public/modelsupport/media/Complete","objective":0},{"inspection_started":"2015-06-04T23:08:47"},"o","n",""]]}
mmundy42 commented 9 years ago

Just to be clear "minfluxsddf" is testing an unsupported parameter?

There is code that is supposed to validate the input parameters but something doesn't seem to be working quite right.

nconrad commented 9 years ago

That's correct.

Since there is no type spec for these methods, I guess it would be safest for me to look at the ProbModelSEED code for testing, taking a close look at the results produced.

mmundy42 commented 9 years ago

I have a fix for the Bio::KBase::ObjectAPI::utilities::ARGS() function to throw an error when there is an unsupported parameter specified. But I'm not sure if it is going to work without some additional changes. For example, adminmode is passed around as a parameter but it is not explicitly listed in the set of valid arguments. See the code block starting at https://github.com/ModelSEED/ProbModelSEED/blob/master/lib/Bio/ModelSEED/ProbModelSEED/ProbModelSEEDHelper.pm#L490. I'm not sure if there are other examples.

I agree with Neal that an unsupported parameter should be flagged as an error. But it might take some work to find all of the places where extra parameters are passed along that don't cause an error now.