action #64075
closed
Use validation consistently in WebAPI controllers
Added by livdywan about 5 years ago.
Updated over 4 years ago.
Category:
Feature requests
Description
While working on client scripts I ran into bugs to do with incorrect use of the API and error handling. So I thought it worth investigating how we currently validate parameters and what could be done to make it more predictable and more reliable.
Retroactively filing this ticket to track my investigation and proposals.
- Status changed from New to In Progress
cdywan wrote:
I chose the JobTemplates controller to add validation in gh#os-autoinst/openQA#2781 to test out my proofs of concept:
- Distinguish missing from invalid parameters
PR has been merged 14 days ago, no problems reported. I assume by now also some users have done enough interaction with the API to not find problems with this. Do you see further places where work needs to be done or are we done here?
- Checklist item changed from to [x] JobTemplates create,update, [x] JobGroup create,update, [x] Asset register,get,delete, [x] Bug list,show,create,update,destroy, [ ] Comment create, [ ] Job list, [x] Locks, [ ] Mm get_children,get_parents, [ ] Worker list,create
- Subject changed from Use validation consistently in controllers to Use validation consistently in WebAPI controllers
To make it clearer what's needed to resolve this, I'm adding a checklist of what's been implemented and what probably should be implemented here.
- Checklist item changed from [x] JobTemplates create,update, [x] JobGroup create,update, [x] Asset register,get,delete, [x] Bug list,show,create,update,destroy, [ ] Comment create, [ ] Job list, [x] Locks, [ ] Mm get_children,get_parents, [ ] Worker list,create to [x] JobTemplates create,update, [x] JobGroup create,update, [x] Asset register,get,delete, [x] Bug list,show,create,update,destroy, [ ] Comment create, [x] Job list, [x] Locks, [ ] Mm get_children,get_parents, [ ] Worker list,create
- Checklist item changed from [x] JobTemplates create,update, [x] JobGroup create,update, [x] Asset register,get,delete, [x] Bug list,show,create,update,destroy, [ ] Comment create, [x] Job list, [x] Locks, [ ] Mm get_children,get_parents, [ ] Worker list,create to [x] JobTemplates create,update, [x] JobGroup create,update, [x] Asset register,get,delete, [x] Bug list,show,create,update,destroy, [x] Comment create, [x] Job list, [x] Locks, [x] Worker list,create
Semi-related, verifying test logs of the validation PRs, I noticed
- test coverage doesn't cover error logging in
lib/OpenQA/WebAPI/Controller/Running.pm
- we have some not so helpful backtrace spew in
lib/OpenQA/Schema/Result/Jobs.pm which would benefit from getting being reduced to the error at hand like we do elsewhere
- Status changed from In Progress to Feedback
- Status changed from Feedback to Resolved
The validation for the 'latest' param for querying jobs seems weirdly strict, since the code simply checks whether it's set to anything at all. I was using 'latest=true' in some code in the Python client, and that has broken on update to recent openQA git because now only 'latest=1' is valid.
Also available in: Atom
PDF