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

[Petr Vobornik]

On 10/23/2012 05:57 PM, Petr Viktorin wrote:
> 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.

Probably the easiest and default solution. I'm also considering 
something similar to 'success notification' - a message displayed in a 
header for a brief period. It may be better because confirming dialog is 
uncomfortable.

>
> ----
>
> 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 Vobornik