import_records requires caller to check for REDCap error codes about pycap HOT 11 CLOSED

I've been thinking about this for a while and what the best behavior should be. Here are some thoughts in no particular order.

• I always take a look at the response from import records API call (at least you'll get a stacktrace somewhere if it errors out):

response = project.import_records(data)
assert 'count' in response and response['count'] == len(data)

• 'count' is always in response when API responds with a 200 status. 'error' is always in the response for errors.
• The response content for errors is helpful for humans, as you show above, but less for machines.
• There's no built-in mechanism in the responses to discover the most-useful keys in response. In your example above, there's nothing explicit about the existence of 'fields' key.

As I'm thinking through this, maybe it's best to raise a RedcapError and populate it with the string representation of the response. For csv and xml formats, this would be the raw response and json responses can be the string rep of the decoded dictionary.

Any thoughts @nicholsn or @aaron0browne?

from pycap.

I haven't used import_records much, but it seems to me that a lot depends on the way REDCap responds to given situations, and mirroring the REDCap responses with project code seems fragile. I think documenting the best practice of checking the response is best. In the given example where some fields are not found, if REDCap imports the rest of the data, then an Error doesn't seem appropriate, because some action was completed, so maybe you want a warning. But are there some situations in which the REDCap aborts the whole import (missing id field, etc), and wouldn't you want an Error for that?

In short, I think the complexity of the error reporting is beyond the scope of needed functionality in PyCap and the user should simply be directed to check the response (which shouldn't be more complicated than handling a generated error, anyway).

Was that too harsh? Sometimes my stream-of-thought tends to extremism...

from pycap.

Thinking about it more...I agree with the both of you. Above all, raising an exception where it wasn't before is a major API change so anything that would come of this will wait till the mythical 1.0 release 😄

That being said, philosophically I think the exception makes sense (explicit is better than implicit), so thanks for the issue @dckc . But like @aaron0browne brings up, I'm not going to chase REDCap error responses so what does get thrown will be very general and serve nothing more than to make stack-traces more readable if you choose to not catch the exception.

Documentation definitely should better address "best-practices" as far as this goes.

from pycap.

typically 1.0 marks API stability, so breaking changes should happen before, not after, 1.0.

see also http://semver.org/

from pycap.

chasing down the 'fields' part is above and beyond the call of duty. What seems right to me is, inside the import_records() implementation:

  if 'error' in response:
    raise IOError(response['error'])

bonus points for a purpose-built subclass of IOError.

from pycap.

typically 1.0 marks API stability, so breaking changes should happen before, not after, 1.0.

So this should come in 0.9 and follow semver. I've got no problem with that. There are other tweaks to arguments I want to make as well.

if 'error' in response:
    raise IOError(response['error'])
bonus points for a purpose-built subclass of IOError.

My thinking was this:

if 'error' in response:
    raise RedcapError(str(response))

This way the other fields are there for human inspection and I don't introduce another exception callers have to look out for.

from pycap.

That looks fine to me.

from pycap.

I've been asked to upload .csv files from my company and I get thrown errors such as these:
The following fields were not found in the project as real data fields: 18119291510791274940
or
The following fields were not found in the project as real data fields: error_message.

I have no idea where to go with this, can anyone assist?

from pycap.

from pycap.

from pycap.

from pycap.