-
Notifications
You must be signed in to change notification settings - Fork 11
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
Caps Disco #136
Caps Disco #136
Conversation
The current |
Concerning apiVersion, IMHO its useful for humans, as the branding, and should NOT be relied upon to decide on caps. Yet it is used for that by Nextcloud. |
Capabilities should be about being able to process something you receive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I think we can include the full list in the spec as per #121 !
@michielbdejong can you please rebase this on top of |
febdb39
to
5ba5b42
Compare
Rebased on develop. |
I think we want to start off our capabilities list with all functionalities that we introduces since 1.0-proposal1 and for which one server needs to know whether the other one supports it. I can think of the following list:
Functionalities which I think don't make sense to announce (these functionalities can just be present unannounced):
|
so caps to add:
webapp URI should probably also somehow be bound to the server domain name from discovery |
Let's not forget:
As very first capability (in the form of endpoint)! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few preliminary comments for some discussion
IETF-RFC.md
Outdated
@@ -234,10 +234,10 @@ Step 7: The JSON response body is the data that was discovered. | |||
The JSON response body offered by the Discoverable Server SHOULD contain the following information about its OCM API: | |||
|
|||
* REQUIRED: enabled (boolean) - Whether the OCM service is enabled at this endpoint | |||
* REQUIRED: apiVersion (string) - The OCM API version this endpoint supports. Example: `"1.1.0"` | |||
* REQUIRED: apiVersion (string) - The OCM API version this endpoint supports. MUST start with `"1."` and clients MUST ignore the rest of the string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As already commented, I'd not enforce any value, in particular the MUST start with 1.
- we will have 2.0 :)
Maybe just It is provided for information purposes only: clients SHOULD NOT infer capabilities based on its value
, or (better) say that when discussing capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My idea was that servers can announce which version of the spec they support. So if they support the version that says "announce me as 1.*", then that is what they should announce.
We could keep it as "anything goes" but we don't want a 1.* server to announce a 2.* API version, right?
we will have 2.0 :)
Exactly, and then we'll change the "MUST start with 1." to "MUST start with 2.", that's how we link the api version to the spec text. We can discuss it in today's meeting!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I'll miss today's meeting)
Hmm I'm not sure - IMHO the spec's version is an attribute, a "payload", not an inherent feature ("MUST start with...").
And whether a server announces version A.B
but actually implements C.D
, this is more a matter of compliance that can (and should) be independently tested, e.g. by (y)our test suite, not of "adhering to a standard" I think.
There are two sides to this and I think neither of them warrant a discoverable capability. When Alice sends an invite to Bob:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Are you sure? The principle of invite is for a user to add a trusted party, even if the Server URL of the other party is well known. I think discovering whether B's server supports acceptance of invites (which ok implies also issuance of invites, both being an optional feature, and thus a capability), even without a WAYF page, is necessary for A's server to expose the invite flow altogether. |
Ok, my understanding is that the two points from @glpatcern is that:
For me I am fine with both these modifications to this pr, and if we do them, we can go ahead and merge this pr which would be a great step onwards to version 1.2. So what do you say @michielbdejong can you agree to those two changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Do you want to patch the spec.yaml
in this PR as well?
Checked that it looks OK on spec.yaml render of this branch: |
In my presentation about OCM at CS3 2023 in Barcelona I had this slide about CAPabilitieS DISCOvery ;)
Now I'm thinking if we put good capability discovery into the
/.well-known/ocm
//ocm-provider
document, then discovery doesn't even have to rely on theapiVersion
field we have there. Older implementations will just have less entries in the capabilities object, or the object will be missing from the document altogether.