OpenField Class

[codygordon]

#1011

cc @TheReFTW - please loop in anyone from your team who should help maintain this!

[TheReFTW]

Cody! This is so exciting!

CCing: @drewnewell, @isik-kaplan

[codygordon]

@sjwmoveon would like your advice on this:

On 400 errors, some OpenField endpoints such as the bulk upsert people return a list of dicts that map to the indexes of the provided list, and will contain the columns containing bad data if there are any.

For example, a parsed response here could look like:
[{}, {'phone1': ['The phone number entered is not valid.']}, {}, {}, {}, {}, {}, {}, {}, {}, {}, {'phone1': ['The phone number entered is not valid.']}, {}, {}, {}]

Right now, this just stringifies that response and adds it as a newline to the Exception message, under the status code.

A user of the connector needs to use that list to identify the problem data and remove it or fix it before retrying the call, and right now I end up needing to process the response like this:

if "400" in str(e):
  parsed_error_rows = ast.literal_eval(str(e).splitlines()[1].strip())
  
  for idx, row in enumerate(parsed_error_rows):
      for key in row:
          # do something with the bad data

Are you aware of a best practice that fits within the standards of how parsons returns errors that we could employ here to bubble both the status code and json in a way that's easier to parse for consumers?

[codygordon]

I think I figured out the optimal way to do this by extending Exception and appending relevant data from the response:
https://github.com/move-coop/parsons/pull/1044/files#diff-14899469f7551d91402e8a36a2d18b2ba509a00a8674092b7b7ab82eb7ef708bR11

This way, the consumer then looks like:

 except FailureException as e:
        if e.status_code == 400 and isinstance(e.json, list):
            for idx, error_row in enumerate(e.json):
                for column_key in error_row:
                    # fix or remove the problem data

[codygordon]

@shaunagm I probably won't be adding more to this, not sure if you want to merge it as-is so it can be added to in the future, or ask someone else to add more to it?

I should be able to find some time to stub out some basic tests for the methods I created here; at the very least, I can confirm these endpoints have been used in production so they are field-tested.

[shaunagm]

@codygordon do you need this merged on any particular timeline? My preference would be to wait until tests are added to merge it, but if you need this merged fast we can make an exception, as the field-testing (which you've done) is the more important part.

[shaunagm]

Needs the docs link to the sidebar, plus ideally tests (see my other comment/question re: your timing - tests are not a dealbreaker if you're in a rush)

[shaunagm]

This looks great but don't forget to add a link to the connector to the sidebar

[shaunagm]

I'm surprised you didn't use the API Connector class as the client here. Is that class not suitable for your purposes, or were you unaware it existed? If you chose not to use it, I'd love to get your feedback on what made it unworkable.

[codygordon]

Just unaware! I can refactor to use it as it seems better to have a standard in place like this now, for sure.

[codygordon]

No rush, will write some tests 👍