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

409 Conflict under 7.1 Error Codes specifically calls out PUT for statements but not POST #362

Closed
bscSCORM opened this issue Jul 17, 2013 · 7 comments
Labels

Comments

@bscSCORM
Copy link
Contributor

POST of statements should also be mentioned here (when posting a batch of statements there can still be a conflict, in which case 409 should still be used)

@andyjohnson
Copy link
Contributor

Fine with this change in patch as sections below clearly reference POST using error code 409.

@garemoko
Copy link
Contributor

Do we also need to look at section 6.3 Concurrency? This currently refers only to PUT (and GET) but not POST.

@andyjohnson
Copy link
Contributor

I'd think we'd only need POST in the context of if it had a GET in it, but I think we don't need to spell that out here. That's my 2 cents but it is definitely worth discussing!

@fugu13
Copy link
Contributor

fugu13 commented Sep 18, 2013

I'm not sure I follow your comment, @andyjohnson ; 409 conflict occurs when something is being updated, so this is for true POSTs that update state & profile.

I'm fine with this, except the correct response is 412 (per the RFC), not 409, which was an error in the spec. See #356

@fugu13
Copy link
Contributor

fugu13 commented Sep 18, 2013

Or rather, both, since it looks like the 409/412 stuff is sorted (and both are potentially appropriate and only fully called out for PUT).

dacdave pushed a commit to dacdave/xAPI-Spec that referenced this issue Nov 19, 2013
It seemed to make sense to bundle this in with other edits in the same area. 

Do we also need to look at section 6.3?
@andyjohnson
Copy link
Contributor

So as a part of 1.0.3, lets rework section 6.3 to take into account these considerations as @garemoko said, sorry for the confusing comment earlier.

@andyjohnson andyjohnson added patch and removed minor labels Oct 2, 2014
@garemoko
Copy link
Contributor

garemoko commented Jan 7, 2015

This has since been resolved by #520 and #564

I believe this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants