[katello-devel] Apipie and validation.

[Dmitri Dolguikh]

On 05/12/12 12:21 PM, Ohad Levy wrote:
>
> ----- Original Message -----
> | On 04/12/12 09:47 PM, Ivan Necas wrote:
> | >
> | > ----- Original Message -----
> | >> On Tue 04 Dec 2012 05:10:28 AM MST, Dmitri Dolguikh wrote:
> | >>> On 03/12/12 09:36 AM, Lukas Zapletal wrote:
> | >>>>> Step #1 - let's get rid of controller validations.
> | >>>> Again, you can only do this for CRUD actions in the controllers.
> | >>>> For all
> | >>>> other methods we need validations. And we have those, not all
> | >>>> our
> | >>>> controllers are plain REST-driven. We do have procedure-like
> | >>>> actions.
> | >>>> Maybe more then we should have, but this is a fact.
> | >>>>
> | >>>> You say all the validations should be done in models, thus you
> | >>>> essentially say let's put all our code logic there. Because it
> | >>>> does not
> | >>>> make sense to do validations in models while doing anything else
> | >>>> (that
> | >>>> also requires some level of validations) in controllers or a
> | >>>> logic
> | >>>> layer. A fact: you need to do validations also outside of
> | >>>> models.
> | >>> Domain logic belongs in the model layer. We can discuss how to
> | >>> better
> | >>> factor model layer code, but not where to put it.
> | >>>
> | >>> -d
> | >>>
> | >>> A lot of rationalizations to keep validations in apipie have been
> | >>> put
> | >>> forward, *all* of them attempting to deal with various smells,
> | >>> code
> | >>> or
> | >>> process ones. Let's fix the root issues.
> | >>>
> | >>> Again, Let's get rid of validations in apipie. Refactor the
> | >>> controllers when needed. If apipie is not useful without those
> | >>> validations, let's suspend/stop use of it.
> | > Am I reading it correctly you suggesting getting rid of API
> | > documentation?
> |
> | Only if it's not useful. Or we can't agree that it's useful. Do we
> | have
> | any consumers for docs atm?
>
> I would argue that generating a client based on the binding (CLI or even katello) should benefit from the validations as it would safe a roundtrip.

I'm not saying we don't need validations. I'm saying that mustn't 
duplicate validations, or constraint them above what model considers 
valid. Violating this results in code duplication and bizarre behaviour 
when server can't consume data it produced. among other things.

As for client generation, I'm *still* not convinced it's worth it in the 
long run, and I'm definitely experiencing more work in the short term: 
atm I need to make changes on the foreman side, make changes in 
foreman_api, generate a new version of the gem, and only then I'll be 
able to test the changes. For something that changes a lot (as foreman 
api currently are) this is hardly optimal, or convenient (esp. taking 
into account that Katello is the only consumer of these api atm).

-d

>
> Ohad
> | -d
> |
> | >
> | > -- Ivan
> | >
> | >>> -d
> | >>>
> | >> +1
> | >>>> It is wrong to put all your code logic into models. And
> | >>>> unfortunately we
> | >>>> have most of it there. Rails follows MVC pattern and what we
> | >>>> have
> | >>>> is
> | >>>> those
> | >>>> messy Models-for-everything approach. If you mix this with our
> | >>>> orchestration, it is deadly combination. Difficult testability,
> | >>>> huge
> | >>>> objects with no responsibility split, unable to mock things and
> | >>>> so
> | >>>> on.
> | >>>> Code logic in models works well for small projects and it is
> | >>>> de-facto
> | >>>> standard in Rails world. People just blindly follow this wrong
> | >>>> pattern.
> | >>>>
> | >>>> So I don't agree with this simplification you offer. We will
> | >>>> likely
> | >>>> start to refactor our orchestration creating new layer of code
> | >>>> logic,
> | >>>> which is the right approach to do things. And we should move
> | >>>> complex
> | >>>> methods like activation key subscription from models there. Then
> | >>>> we will
> | >>>> need validations on the controller side (or on the newly created
> | >>>> logic
> | >>>> layer or orchestration layer - depends on how are we gonna name
> | >>>> it).
> | >>>>
> | >>>> So yes, it has something to do with REST. My counter proposal
> | >>>> is:
> | >>>>
> | >>>> #2 Let's get rid of validations where they duplicate model
> | >>>> validators,
> | >>>> but let's keep them for (usually non-CRUD) methods which need
> | >>>> then. We
> | >>>> will likely be extending those soon.
> | >>>>
> | >>> _______________________________________________
> | >>> katello-devel mailing list
> | >>> katello-devel at redhat.com
> | >>> https://www.redhat.com/mailman/listinfo/katello-devel
> | >>
> | >>
> | >> --
> | >> Jason E. Rist
> | >> Senior Software Engineer
> | >> Systems Management and Cloud Enablement
> | >> Red Hat, Inc.
> | >> +1.919.754.4048
> | >> Freenode: jrist
> | >> github/identi.ca: knowncitizen
> | >>
> | >> _______________________________________________
> | >> katello-devel mailing list
> | >> katello-devel at redhat.com
> | >> https://www.redhat.com/mailman/listinfo/katello-devel
> | >>
> |
> | _______________________________________________
> | katello-devel mailing list
> | katello-devel at redhat.com
> | https://www.redhat.com/mailman/listinfo/katello-devel
> |