Configurable cull arbitration

[rcthomas]

Here's a proposal at making configurable the decision about whether or not to cull a server. The idea is to provide a hook that can be configured that takes 3 arguments. The first 2 are the usual inputs to decide whether or not to cull --- the idle time of a server and the idle time limit. The third argument is the server dict. Default cull arbitration is the familiar:

def default_cull_arbiter(inactive, inactive_limit, server):
    """Return True if time inactive exceeds limit, the classic cull check."""
    return inactive.total_seconds() >= inactive_limit

But now the user can do this in a traitlets config file since #83 is merged:

def my_cull_arbiter(inactive, inactive_limit, server):
    if server['state']['profile_name'] == 'unlimited':
        return False
    return inactive.total_seconds() >= inactive_limit
c.IdleCuller.cull_arbiter_hook = my_cull_arbiter

This exempts servers with a profile name of "unlimited" from ever being culled. For this to work the cull arbitration function needs to "know" about the kinds of stuff it can check in the server dict. Some other changes and things to note:

• Imports the Callable traitlet; if the cull_arbiter_hook is left as None then the default behavior is used
• If the user wants to be able to re-use the idle timeout comparison, there might be better ways than saying "always remember to include this return inactive.total_seconds()... line. But the user still has to know they need to either include that check or somehow tell the culler to do it in addition to (or instead of) whatever logic they've got.
• Removes some comments advising on how to customize logic via patching the code
• Should be documented, however this ends up getting done

Questions:

• Do people like this or not?
• What kind of tests does this need?
• The comments about how to customize through patching the code opened the door to even more arbitrary changes but I decided not to enable those suggestions there --- this just checks the server state.

Addresses #9, #25

[manics]

I haven't thought too deeply about which parameters to pass, but providing the configurability requested in those linked issues through a hook is a great idea, and involves adding minimal complexity to this repo.

[manics]

Another thought is to state that hooks must take arguments as keywords, and should ignore unknown **kwargs to allow for future extensibility. We haven't made this requirement for hooks in other JupyterHub projects, but since Traitlets are new here I think it's fine to change.

[rcthomas]

@manics could you elaborate on that "must" requirement --- is that a "must only" and can you point to an example implementation that looks like what you're aiming for? I don't think it's a problem to make a change like that here.

[manics]

We can only document it as part of the contract, and ensure we follow it in our own default hooks by changing default_cull_arbiter to accept keyword-only arguments:

def default_cull_arbiter(*, inactive, inactive_limit, server, **kwargs)
    ...

We can't enforce it, but by documenting it we can easily add more parameters in future without it being a breaking change.