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

Can we drop singleProtocolNew? #125

Closed
michielbdejong opened this issue Sep 6, 2024 · 3 comments · Fixed by #131
Closed

Can we drop singleProtocolNew? #125

michielbdejong opened this issue Sep 6, 2024 · 3 comments · Fixed by #131

Comments

@michielbdejong
Copy link
Contributor

michielbdejong commented Sep 6, 2024

Related to #104.

Looking at the API spec there are 3 ways to specify that WebDAV and nothing else is offered:

  1. name: 'webdav', details in 'options' (singleProtocolLegacy)
  2. name: 'webdav', details in 'webdav' (singleProtocolNew) -> why do we need this?
  3. name: 'multi', details in 'webdav' (multipleProtocols)

Can we remove the second one?
Note that if we decide to drop name in #104 then 2. and 3. automatically become identical.

@glpatcern
Copy link
Member

The case 2. was provided as an example of an implementation that would use the "new" format even for the case where the offered protocol is a single one (I'm sure we can find some relevant discussion in the PRs from last year - and yes this would be granted an ADR record!). Reva is one such implementation where we went straight for the new format in all cases.

@michielbdejong
Copy link
Contributor Author

But then why do you set name to webdav and not to multi?
If you set it to multi then it would be correct, the receiving implementation would just loop over the keys of the object, for this loop it doesn't matter if it's 1,2 or 3 entries.
But by setting name to webdav you're forcing the receiving implementation to add an extra if statement to treat name webdav as equivalent to name multi.

@glpatcern
Copy link
Member

That's a fair point indeed, and in Reva we did as you suggested: multi is "hardcoded" and we populate the protocols as requested or configured.
Shall we just modify the singleProtocolNew example to say multi? Still better than dropping it completely I guess.

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

Successfully merging a pull request may close this issue.

2 participants