I've created BP on API documentation: https://blueprints.launchpad.net/fuel/+spec/documentation-on-rest-api-input-output
AFAIC, https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-testing can be done in parallel with all other things. And we could add new validators tests as unit tests right now. Aleksey Kasatkin S. Software Developer | Mirantis, Inc. | http://www.mirantis.com cell: +380938330852 | skype: alexeyk_ru On Tue, May 20, 2014 at 6:48 PM, Igor Kalnitsky <[email protected]>wrote: > Aleksey, > > As I've pointed early, we have a blueprint to write unit-tests for > validators, not integrational. It's a good idea, I think. If we write more > tests now (as integrational), we will write the same tests later as > unit-tests. > > So my point is: if the > blueprint<https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-testing>will > milestoned for 5.1, i propose to write more tests after it will be > completed. > > > - Igor > > > > On Tue, May 20, 2014 at 5:48 PM, Aleksey Kasatkin > <[email protected]>wrote: > >> Igor, >> >> AFAIC, validators refactoring is another task that should be done in >> parallel. >> >> In my opinion, documenting of API (parameters JSONs) should be done as a >> first step to improve input cases coverage. >> AFAIK, we have the same task for the orchestrator: there is no >> documentation on its input yet (just some fragments and samples). >> >> >> Aleksey Kasatkin >> >> S. Software Developer | Mirantis, Inc. | http://www.mirantis.com >> cell: +380938330852 | skype: alexeyk_ru >> >> >> On Tue, May 20, 2014 at 3:50 PM, Igor Kalnitsky >> <[email protected]>wrote: >> >>> Hi guys, >>> >>> Indeed, our validators don't cover all input cases and it's bad. >>> Moreover, our validators is too complicated to support and we have some >>> plans to refactor them. >>> >>> Here the blueprints we have: >>> >>> - >>> https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-to-objects >>> - https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-testing >>> - https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-json >>> >>> So.. In order to reduce our job, I propose to implement this >>> blueprint<https://blueprints.launchpad.net/fuel/+spec/nailgun-validators-testing>first, >>> and then to cover input cases as much as possible. >>> >>> What do you think about it? >>> >>> - Igor >>> >>> >>> On Tue, May 20, 2014 at 3:30 PM, Aleksey Kasatkin < >>> [email protected]> wrote: >>> >>>> Vitaly, >>>> That's all about validation of input data indeed. There is no sense to >>>> validate own output. >>>> You had proposals about declarative description of validation logic in >>>> April in Moscow. Can you provide links to materials on this topic? >>>> >>>> >>>> Aleksey Kasatkin >>>> >>>> S. Software Developer | Mirantis, Inc. | http://www.mirantis.com >>>> cell: +380938330852 | skype: alexeyk_ru >>>> >>>> >>>> On Tue, May 20, 2014 at 3:10 PM, Vitaly Kramskikh < >>>> [email protected]> wrote: >>>> >>>>> Aleksey, >>>>> >>>>> As for that particular bug, there was an UI issue, so UI sent disk >>>>> names which didn't exist on a node and backend code didn't handle this >>>>> case >>>>> properly, so that caused 500 error. It has nothing to do with validation >>>>> of >>>>> user input, it is a check of existence of every model mentioned in a >>>>> request and it must always be performed on backend. UI and CLI have >>>>> nothing >>>>> to validate, as they construct this data, and UI was constructing it >>>>> improperly. We had/have cases where absence of this kind of validation >>>>> leads to 500 error on every request to node handler, so nailgun becomes >>>>> unusable. >>>>> >>>>> As for validation of user input, I think we need to describe >>>>> declaratively as many validation rules as possible to minimize amounts of >>>>> validation logic in backend and UI. Refactoring of networks is a good >>>>> example - now we have a set of flags for every network so size of >>>>> validation code reduced dramatically and we don't have hardcoded network >>>>> names anywhere. The same can be done for roles: >>>>> https://review.openstack.org/#/c/89601/6/nailgun/nailgun/fixtures/openstack.yaml. >>>>> So UI and backend should use these configs to validate data. Also complex >>>>> validation logic which cannot be described declaratively should be >>>>> validated on backend only (like intersection of networks). >>>>> >>>>> I think CLI should not have any validation at all and just display >>>>> server response in case of 4xx error. >>>>> >>>>> >>>>> 2014-05-20 13:56 GMT+04:00 Aleksey Kasatkin <[email protected]>: >>>>> >>>>> Yes, the validation of input data in Nailgun doesn't cover all >>>>>> possible data integrity problems. We just send data from UI and CLI using >>>>>> right format. It's true for networking data as well (e.g. networks roles >>>>>> are not validated, networks can be duplicated). >>>>>> AFAIC, it's very simple to get 500 error using curl. Data validation >>>>>> in Nailgun should be strengthened if we want to get the same >>>>>> usability for API as for UI and CLI. >>>>>> It is partly implemented with introduction of JSON schemas for >>>>>> objects. Additional validation logic should be added, especially for >>>>>> complex data sets like disks and networking data. >>>>>> We enhance data validation occasionally but API is not documented >>>>>> enough (no description on parameters JSONs). We should get it covered >>>>>> with >>>>>> documentation at first and make proper validation according to that doc >>>>>> then (as it was with Networking limitations: see >>>>>> https://etherpad.openstack.org/p/limitations-of-networking-configuration). >>>>>> >>>>>> Validation for CLI can be done similarly. >>>>>> >>>>>> Vitaly, please comment on this problem from UI point of view. What >>>>>> can be done to formalize validation in UI and to make it systematic? >>>>>> >>>>>> >>>>>> Aleksey Kasatkin >>>>>> >>>>>> S. Software Developer | Mirantis, Inc. | http://www.mirantis.com >>>>>> cell: +380938330852 | skype: alexeyk_ru >>>>>> >>>>>> >>>>>> On Mon, May 19, 2014 at 3:35 PM, Ivan Kolodyazhny < >>>>>> [email protected]> wrote: >>>>>> >>>>>>> Hi Fuel devs! >>>>>>> >>>>>>> I'm looking on fix https://bugs.launchpad.net/fuel/+bug/1319306. >>>>>>> This bug is fixed only for Web UI. But it should be fixed on back-end >>>>>>> site >>>>>>> too. Let's add validation for it. AFAIK, at least we've got similar >>>>>>> errors >>>>>>> with roles assignment validation earlier. >>>>>>> >>>>>>> I agree, that we need as much as possible validations on our Web UI. >>>>>>> But back-end validation is very important too. It must be implemented >>>>>>> because Web UI is only one of possible users of our API. A good REST API >>>>>>> must not raises 500 errors. 4xx HTTP status codes should be used for >>>>>>> any >>>>>>> case of invalid data. It's a good practice and it makes API usage less >>>>>>> risky. >>>>>>> >>>>>>> I propose to implement validation on every API client and API >>>>>>> handlers: >>>>>>> >>>>>>> - API handlers - all input data must be validated; >>>>>>> - Web UI - all input data should be validated as much as >>>>>>> possible; >>>>>>> - Fuel (CLI) Client - at least, all required parameters should >>>>>>> be validated. >>>>>>> >>>>>>> >>>>>>> Let's discuss my proposals and file bugs and blueprints if needed. >>>>>>> >>>>>>> -- >>>>>>> Regards, >>>>>>> Ivan Kolodyazhny, >>>>>>> Software Engineer, Mirantis, Inc. >>>>>>> >>>>>>> -- >>>>>>> Mailing list: https://launchpad.net/~fuel-dev >>>>>>> Post to : [email protected] >>>>>>> Unsubscribe : https://launchpad.net/~fuel-dev >>>>>>> More help : https://help.launchpad.net/ListHelp >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Vitaly Kramskikh, >>>>> Software Engineer, >>>>> Mirantis, Inc. >>>>> >>>> >>>> >>>> -- >>>> Mailing list: https://launchpad.net/~fuel-dev >>>> Post to : [email protected] >>>> Unsubscribe : https://launchpad.net/~fuel-dev >>>> More help : https://help.launchpad.net/ListHelp >>>> >>>> >>> >> >
-- Mailing list: https://launchpad.net/~fuel-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~fuel-dev More help : https://help.launchpad.net/ListHelp
