On 10/29/2012 09:22 AM, Jan Cholasta wrote:
On 26.10.2012 16:35, Petr Viktorin wrote:
On 10/25/2012 04:55 PM, Jan Cholasta wrote:
Hi,
On 23.10.2012 17:57, 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.
I think this has potential to break stuff. An old client unaware of
capabilities might not send version and as a result receive unexpected
data, which might trigger some error.
Our client always sends the version. We already use it to detect clients
that are too old or too new. Clients that do not send it are not
protected from breakage. They are not using the API correctly.
Also, if any other clients currently exist, they're unlikely to validate
the results as strictly as we do, so an extra "warnings" entry shouldn't
hurt them.
It's better to be safe than sorry, IMHO.
What I'm proposing is just adding a warning when the client doesn't call
the API correctly (it did happen to work before if you were lucky --
there was just no protection from incompatible server versions).
I really don't see how this could cause any major trouble.
And I like easy RPC calls too much :)
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"))
Here's a couple of things to consider:
* There should be an API that doesn't require the version and result
arguments, because they might not be at hand when a warning needs to be
issued (e.g. in pre/post command callbacks or utility functions).
I don't see how this can be done without introducing global/per-thread
state or reworking the system.
We already use per-thread state frequently, we can use it here as well.
I'm not sold.
Anyway, I think this this is outside the scope of this RFE. It can be
implemented separately when it's needed, on top of the proposed API.
Since it doesn't change the external API we can easily break it into a
separate effort.
Not having access to the output is a limitation of the callbacks. IMO
it's a symptom of a deeper problem: they don't have access to any state
other than what the current callbacks need, and we can't easily make
more stuff available to them.
I think this problem is orthogonal to this RFE and should be solved
separately.
(Note: the version is part of options, so callbacks do have acccess to
it.)
* There should be more message levels than just "warning". I think it
should be at least "error", "warning", "info".
I have included "error" here, because currently we can't send more
than one error back to the client, but it might be handy:
# ipa command --opt1 ugly --opt2 uglier
ipa: ERROR: invalid 'opt1': must be an integer
ipa: ERROR: invalid 'opt2': must be at least 10 characters
Unfortunately, XML-RPC doesn't allow any extra data in error messages,
it's just errno and text. So we can't give multiple errors to the client.
XML-RPC is not the only transport mechanism we have, so I don't think
this XML-RPC-centric thinking is appropriate. If XML-RPC does not have
native support for this, we certainly can do it some other way.
Regarding the "info" level, I don't see a use for it. Information is
what the result dictionary already contains.
Debugging comes to mind. My point is that this thing should be
extensible, so should we require a new message category in future, it
can be added easily.
OK, then. In this case I should rename "warning" to "message" in the
whole RFE.
Does "warning", "info", and "debug", the latter displayed only the "-v",
resp. "-d" options, sound good?
Plus we'd have "error" and "critical" (unused for now because XML-RPC
doesn't allow extra error data).
Unfortunately, new categories can't be added too easily. Clients would
have to be taught what to do with the new message types.
--
Petr³
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
