[katello-devel] Foreman-api

[Dmitri Dolguikh]

On 28/08/12 01:27 PM, Chris Alfonso wrote:
> On 28/08/12 10:22 +0100, Dmitri Dolguikh wrote:
>> On 27/08/12 10:27 AM, Ivan Nečas wrote:
>>> On 08/24/2012 04:58 PM, Dmitri Dolguikh wrote:
>>>> On 24/08/12 03:57 PM, Bryan Kearney wrote:
>>>>> On 08/24/2012 10:52 AM, Jason Rist wrote:
>>>>>> On 08/24/2012 10:50 AM, Dmitri Dolguikh wrote:
> [snip]
>>> [1] - 
>>> https://github.com/calfonso/deadwood/blob/master/lib/deadwood/model/environment.rb
>>> [2] - 
>>> https://github.com/mbacovsky/foreman_api/blob/master/lib/foreman_api/resources/architecture.rb
>>
>> I'd argue that the attribute filtration doesn't belong in [1].
> Where does it belong?  If the server rejects certain attributes, the 
> client
> must not allow those attributes to be part of the request.  The 
> workflow is
> as follows..user makes a request to GET an object from the server, the
> returned object has all the attributes that describe the object. The user
> then updates one attribute and then saves the object.  That save 
> action is
> going to generate a PUT request, note: the object still has all the
> attributes that describe the object, but a subset of those attributes 
> are not
> allowed on PUT requests.  I don't think it's reasonable to have the user
> remove the attributes, so I remove them in the client api gem. The 
> user then
> never has to worry about what attributes are allowed/disallowed.
>
> If the attribute removal doesn't belone in the client api gem, where 
> does it
> belong?  The only other option is to have the server ignore those 
> attributes
> rather than return an error code.

It certainly belongs on the client, there is no question about that in 
my mind. I'm not sure that silently dropping an attribute from the hash 
is a good approach though, as it may lead to a confusing situation when 
a developer attempts to update an attribute, sees a success, but the 
attribute hasn't been actually changed on the server.

Haven't looked at how Aeolus/Conductor handles remote models though, 
perhaps this is appropriate there.

Cheers,
-d

>
> - Chris
>> [2] has a lot of repetitive code, indeed that's the reason why you 
>> generate it, and why I don't like it. It makes more sense for 
>> non-standard methods, but index, show, update, create can either be 
>> moved into some common mixin, or be generated similarly to accessor 
>> generators.
>>
>> -d
>>
>