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

Catches KeyError on the properties list when there is no info for the document #101

Merged

Conversation

flavioamieiro
Copy link
Member

I've decided to catch this in the serializer and keep the KeyError in
StoreProxy when there is no '_properties' for the document in mongodb because
there is a difference between having an empty list of properties and having no
information for that document (even though that difference is not important to
the user that is trying to list the document properties in the web app).

Closes #99

… document

I've decided to catch this in the serializer and keep the KeyError in
StoreProxy when there is no '_properties' for the document in mongodb because
there is a difference between having an empty list of properties and having no
information for that document (even though that difference is not important to
the user that is trying to list the document properties in the web app).

Closes NAMD#99
@flavioamieiro
Copy link
Member Author

Returning an empty list seems like a better option than returning 404 or something like that since the document exists in our database, but there are no properties (yet?) for it. We do need to have a status indication on the document itself, but I don't think this should be available in the properties list (i.e.: "/document//status/" make more sense than "/document//properties/status/").

@turicas
Copy link
Contributor

turicas commented Nov 6, 2013

Two comments:

  • About status of processing: I prefer using HTTP header for status instead of a new HTTP resource, since what you are showing is just a status about that resource.
  • About status code: in the case of empty _properties or no _properties, what do you think in returning a HTTP 202 Accepted as I said in Avoid KeyError when there are no analysis ready for a document #99?

@flavioamieiro
Copy link
Member Author

I think returning 202 is a good idea, but to be compliant with the RFC we should first implement the status indicator:

"The entity returned with this response SHOULD include an indication of the request's current status and either a pointer to a status monitor or some estimate of when the user can expect the request to be fulfilled."[1]

Since we already know we need this status indicator, we have no valid reason not to return it with the entity in the 202 response. I've created #102 for this.

[1] http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.2.3

turicas added a commit that referenced this pull request Nov 6, 2013
…erties

Catches KeyError on the properties list when there is no info for the document
@turicas turicas merged commit 56bbd3c into NAMD:develop Nov 6, 2013
@flavioamieiro flavioamieiro deleted the fix/keyerror_on_empty_properties branch November 6, 2013 15:31
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.

Avoid KeyError when there are no analysis ready for a document
2 participants