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

Added comment about specific version requests being optional #90

Merged
merged 3 commits into from
Mar 25, 2021

Conversation

josephg
Copy link
Contributor

@josephg josephg commented Feb 16, 2021

The current spec implies the server must either support requesting all historical versions or none of them. We should make it clear servers can support all, none or some historical version requests based on implementation details.

My language here feels awkward.

@toomim
Copy link
Member

toomim commented Feb 16, 2021

@josephg Please also add yourself as an author to the spec once you've made a change, following the instructions at https://github.com/braid-work/braid-spec#contributing

@toomim toomim added this to the Braid-HTTP 03 milestone Feb 26, 2021
@josephg
Copy link
Contributor Author

josephg commented Mar 9, 2021

I've updated the wording to this:

A server does not need to honor historical version requests for all
documents, for all history. If a server no longer has the historical
context needed to honor a request, it may respond using the 416 Range
Not Satisfiable status code. (Defined in RFC7233, Section 4.4)

With an example:

      Request:

         GET /chat
         Version: "ej4lhb9z78"

      Response:

         HTTP/1.1 416 Range Not Satisfiable
         Version: "ej4lhb9z78"

I'm not actually sure that 416 is the right error code - but I think its a reasonable choice. 404 will cause problems with caching proxies, since it implies the specific requested document no longer exists. 416 responses usually name the invalid range. From the spec in RFC7233:

     HTTP/1.1 416 Range Not Satisfiable
     Date: Fri, 20 Jan 2012 15:41:54 GMT
     Content-Range: bytes */47022

... So I think its reasonable to extend the semantics to instead specify the version from which historical data is not available.

@josephg
Copy link
Contributor Author

josephg commented Mar 22, 2021

Concerns:

  • 416 might not make sense given the request wasn't a range request
  • Alternate: Add a new status code - but that might be difficult to get by HTTPBIS
  • Alternate: 425 Too Early ? 412 Precondition Failed?

Path forward: Change 416 to XXX and merge before 03. We will decide an error status code in a subsequent issue / discussion.

Also: What should the client do in situations like this?

@josephg
Copy link
Contributor Author

josephg commented Mar 25, 2021

I've left the error code open for now, as discussed at the meeting. We will need to revisit this at some point in the future.

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 this pull request may close these issues.

3 participants