[Freeipa-devel] [RFE] Warnings and client capabilities (Was: [PATCH] 0062 Don't crash when server returns extra output)

[Petr Viktorin]

Here is a draft design document for ticket 2732.
Please comment on both the feature itself, and on how to write design 
documents.
Petr¹, please add how the UI should handle this.

== Ticket summary ([https://fedorahosted.org/freeipa/ticket/2732 #2732]) ==

Currently the only way to display a warning on client is to raise 
NonFatalError. This is not particularly good, as it mutes normal command 
output and only one warning can be displayed at a time.

Provide a mechanism for displaying arbitrary number of warnings and 
other messages on clients along with the normal command output.

== Additional problem ==

The client validates the response it receives from the server. If it 
gets any extra items, the validation fails. Relaxing our validation is 
not an option. To support older clients, we need a mechanism for 
backwards-incompatible extensions to the API.

== Solution ==

=== Backend ===

Introduce a "capability" mechanism for backwards-incompatible API 
changes. The first capability will be "warnings": a client with this 
capability can get an extra key in the response dictionary, "warnings", 
which contains a list of non-fatal error messages.

Capabilities are determined by API version. The version is linear; it is 
not possible for a client to support any custom subset of capabilities.

If a client does not send an API version number, we will assume this is 
a testing client (such as a curl from the command line). Such a client 
is assumed to have all capabilities, but it will always receive a 
warning saying that forward compatibility is not guaranteed if the 
version number is not sent.

Capabilities will be recorded in API.txt. When a new one is added, the 
API version must be incremented.

All Commands will be updated to explicitly list 'version?' as one of 
their options in API.txt (most already do, and all take it).

A missing version option will be set as part of validation/filling in 
defaults, so execute() will always get it.

Helper methods will be available for checking a capability and for 
adding a warning to the result:

     add_warning(version, result, _("What you're doing is a bad idea"))

will be equivalent to:

     if client_has_capability(version, 'warnings'):
         result.setdefault('warnings', []).append(_("What you're doing 
is a bad idea"))


=== Frontend ===

In the CLI, warnings will be printed after the normal command 
output.Example (from [https://fedorahosted.org/freeipa/ticket/2563 #2563])

   # ipa dnsrecord-add --ttl=555 localnet r1 --txt-rec=TEST
     Record name: r1
     Time to live: 555
     A record: 1.2.3.4
     TXT record: TEST
    Warnings:
        It's not possible to have two records with same name and 
different TTL values.

The Web UI will display warnings in a modal message box.

----

On 06/20/2012 03:57 PM, Simo Sorce wrote:
> On Wed, 2012-06-20 at 12:47 +0200, Martin Kosek wrote:
>> On Wed, 2012-06-20 at 11:10 +0200, Petr Viktorin wrote:
>>> On 06/12/2012 02:39 PM, Petr Viktorin wrote:
[...]
>>>
>>>
>>> We decided off-list that relaxing validation is not the right thing to do.
>>> A better approach would be to notify the server that the client can
>>> accept extended data (through a header or a version parameter).
>>> So, ticket 1721 is invalid, but we need a better solution to make
>>> https://fedorahosted.org/freeipa/ticket/2732 "Provide means of
>>> displaying warning and informational messages on clients" possible.
>>>
>>> I think that using the existing "version" parameter (which gets added to
>>> RPC calls automatically) would be perfect for this.
>>
>> I agree, API version is exactly what we want. We should not care about
>> client version or if the client is in Fedora, RHEL or Ubuntu.
>>
>>>
>>> Simo mentioned that we don't want to make the API depend on the version
>>> of our client version, so other clients don't need to copy our
>>> versioning scheme.
>>>
>>> However, in the version argument we send the API version, not our client
>>> version. I think other clients should know and advertise what API
>>> version they are using, and the number shouldn't be specific to our client.
>>> It's the perfect place to learn the client's capabilities from, if we're
>>> okay with a linear evolution of the API (as opposed to the client
>>> advertising individual features).
>>>
>>> Simo, can you comment? Hopefully I didn't mishear anything on the meeting.
>>>
>>
>> The biggest asset about API version is that we already have this number
>> available for clients that were already released, we don't have to
>> backport anything.
>>
>> I would keep linear evolution of the API version number as is, but it
>> would be also good to assign new API capabilities with the number and
>> have a simple way of checking if client has the capability, i.e.
>> something like this:
>>
>> def post_callback(self, ..., *keys, **options):
>>      if 'warnings' in version.client_capabilities(options['version']):
>>          send_warning('forward record added, but reverse zone not found')
>>          continue
>>      else:
>>          raise errors.NonFatalError(...)
>>
>> Martin
>
>
> Given the discussion, I guess this is the best option we have right now.
>
> Simo.
>


-- 
Petr³