Skip to content

Tasks should be transmitted with DELETE requests, too#227

Merged
samoht merged 1 commit into
mirage:masterfrom
dsheets:http-delete-task
Jul 14, 2015
Merged

Tasks should be transmitted with DELETE requests, too#227
samoht merged 1 commit into
mirage:masterfrom
dsheets:http-delete-task

Conversation

@dsheets
Copy link
Copy Markdown
Member

@dsheets dsheets commented Jul 11, 2015

Without this, delete requests over HTTP use the previous store task
which is often the server store creation task. Requires mirage/ocaml-cohttp#383.

Some of the code in Irmin_http_server common to POST and DELETE processing could be factored.

@samoht
Copy link
Copy Markdown
Member

samoht commented Jul 11, 2015

a workaround is to not use DELETE and use POST everywhere ...

@dsheets
Copy link
Copy Markdown
Member Author

dsheets commented Jul 11, 2015

It is true that POST could be used everywhere but the protocol semantics are slightly different. In particular, DELETE is idempotent whereas POST is not. I'm not sure it really matters but using DELETE for deletion makes sense and would be nice.

What would you like to do?

@samoht
Copy link
Copy Markdown
Member

samoht commented Jul 11, 2015

Depends if we can have a quick point release for cohttp or not. But I'm fine to use pinning.

Without this, delete requests over HTTP use the previous store task
which is often the server store creation task. Requires mirage/ocaml-cohttp#383.
@dsheets dsheets force-pushed the http-delete-task branch from f3b8f54 to 6cf6869 Compare July 11, 2015 22:10
@samoht samoht mentioned this pull request Jul 11, 2015
samoht added a commit that referenced this pull request Jul 14, 2015
Tasks should be transmitted with DELETE requests, too
@samoht samoht merged commit aad4584 into mirage:master Jul 14, 2015
@samoht
Copy link
Copy Markdown
Member

samoht commented Jul 14, 2015

Thanks!

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.

2 participants