fuel-dev team mailing list archive

[Aleksey Kasatkin <akasatkin@xxxxxxxxxxxx>]

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 <ikalnitsky@xxxxxxxxxxxx>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 <akasatkin@xxxxxxxxxxxx>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 <ikalnitsky@xxxxxxxxxxxx>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 <
>>> akasatkin@xxxxxxxxxxxx> 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 <
>>>> vkramskikh@xxxxxxxxxxxx> 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 <akasatkin@xxxxxxxxxxxx>:
>>>>>
>>>>> 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 <
>>>>>> ikolodyazhny@xxxxxxxxxxxx> 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     : fuel-dev@xxxxxxxxxxxxxxxxxxx
>>>>>>> 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     : fuel-dev@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~fuel-dev
>>>> More help   : https://help.launchpad.net/ListHelp
>>>>
>>>>
>>>
>>
>