Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Add "capabilities" section to version command output #116

Closed
wez opened this issue Jun 30, 2015 · 0 comments
Closed

RFC: Add "capabilities" section to version command output #116

wez opened this issue Jun 30, 2015 · 0 comments

Comments

@wez
Copy link
Contributor

wez commented Jun 30, 2015

Keeping in mind our versioning and compatibility scheme, clients are better off interrogating a set of capabilities rather than performing version number checks.

I'm proposing that we augment the version command so that it outputs something like this:

{
    "version": "3.5.0",
    "capabilities": ["relative-roots"]
}

Including the capabilities list in the version command allows clients to query the version number and obtain the capability set in a single round trip. If we were to add a separate command just for capabilities, clients would need to query the version number to determine if they could then query for capabilities.

As part of this we should define the capabilities. I'm inclined to just have relative-roots as the first and only listed capability initially. All other functionality is effectively and implicitly available if the capabilities field is present in the version output.

I'd like to define relative-roots as complete support for watch-project with the relative_root parameter for the query, trigger and subscription subsystems.

An additional command that we can add would be to help check compatibility, so for example, the client would do:

["capabilities", {"required": ["relative-roots"], "optional": ["pcre"]}]

The response would look something like:

{
   "version": "3.7.0",
   "capabilities": {
       "relative-roots": true,
       "pcre": false
   }
}

If any of the capability names in the required field are missing, the capabilities command would add an error field listing the missing capabilities, causing the command to error out. The capabilities map has boolean values that indicate whether a given capability is supported.

Thoughts?

wez added a commit that referenced this issue Aug 15, 2015
Summary:
to make it easier for clients to optionally take advantage of new
features, this diff introduces the concept of capability names.

We add an optional second parameter to the `version` command that allows the
client to specify a list of optional and/or a list of required capability
names.  It is desirable to piggy back this on a version number check to allow
just one request/response round-trip to be issued to determine version
compatibility.  Thankfully, the version command from older servers doesn't
perform any rigorous checks on the number of parameters, so the extended
capabilities argument will be silently ignored.

The `version` command response will include a `capabilities` hash keyed by the
requested capability names with boolean values set to indicate whether the
server supports the named capability.

If a capability name is listed in the required set and the server does not
support it, an error response is returned.

The python and node clients are extended to add a `capabilityCheck` method.
The implementation adds some simple support for use against a server that does
not support capabilities; for a handful of capability names we know which
versions introduced them.  This allows clients to transition to using
capabilities before all servers have been upgraded.

Refs: #116

Test Plan: `make integration`

Reviewers: amasad, sid0

Reviewed By: sid0

Subscribers: stefanpenner

Differential Revision: https://reviews.facebook.net/D43935
@wez wez closed this as completed Aug 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant