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
