On Wed, Jan 15, 2014 at 05:20:58PM -0500, Yassir Elley wrote:
In looking through the sssd code, there are instances where an
asynchronous operation requires multiple tasks to be done in series (not to be confused
with a parent request with multiple child requests). An example of a series of such tasks
would look like:
* send()
* task1()
* task2()
* done()
One word of warning about the tevent_req style. Some old code in the
SSSD predates this style and both the functions and the accompanying
structure can be named differently (the structure typically wouldn't be
a *_state but rather a *_ctx).
While one would expect that the tasks would be named so as to describe
their behavior, they are typically named as completing a previous task,
giving no indication what the task is going to do. This can be confusing
when reading the logs. For example, if the problem is that kinit failed,
an entry in the log might have the function name "sdap_cli_rootdse_done",
which doesn't help describe the kinit problem (the call stack below shows
how "sdap_cli_rootdse_done" calls "sdap_cli_kinit_step").
In the following call stack, I have used the "bad name" annotation for
those function names that don't describe the function behavior. I have
also suggested using a more descriptive name that describes the functions
behavior (each using a <suffix>). I'm not sure what the <suffix> should
be;
maybe "task" or "phase" or something.
sdap_cli_connect_send
sdap_cli_resolve_next
sdap_cli_resolve_done bad name; sdap_cli_connect_<suffix> would be
better (maybe sdap_cli_connect_task)
sdap_connect_send
sdap_cli_connect_done bad_name; sdap_cli_rootdse_<suffix> would be
better
sdap_cli_rootdse_step
sdap_cli_rootdse_done bad_name; sdap_cli_kinit_<suffix> would be better
sdap_cli_kinit_step
sdap_cli_kinit_done bad_name; sdap_cli_auth_<suffix> would be better
sdap_cli_auth_step
sdap_cli_auth_done bad_name; sdap_cli_connect_done would be better
tevent_req_done
In addition to being non-descriptive, the "done" suffix is currently used in
two different ways, which can be confusing:
* as shown above, "done" is used for a series of intermediate tasks, whose
names are based on previous tasks
* in a simpler scenario (with no intermediate tasks), "done" is used to signify
the final task (which makes sense); for example:
sdap_id_op_connect_send
sdap_id_op_connect_step
sdap_id_op_connect_done
To resolve both issues, I propose we improve our naming conventions by:
* coming to consensus on a suffix for a series of intermediate tasks
I'm not sure we can force a common suffix everywhere. I think this
should be one of 'use your best judgement' cases. Maybe it would make
sense to use past tense when the function ends with a verb (ie
sdap_cli_connected) ?
* declaring "done" to be used only for a final task
(whether in a basic scenario, or a scenario with multiple intermediate tasks)
What do others think?
I think the most important part of the tevent_req style is to have a
common prefix and use it for all of _send/_done/_recv:
struct $prefix_state {
};
$prefix_send();
$prefix_done();
$prefix_recv();
The intermediate 'steps' I'm not sure about, I don't think that needs to
be formally codified.