rbw / aiosnow

Asynchronous ServiceNow Library
MIT License
74 stars 12 forks source link

Datetime() validation exception if 'resolved_at' value is empty #33

Closed sulbig closed 4 years ago

sulbig commented 4 years ago

If I include the 'resolved_at' field in my Incident schema, and make a selection that includes unresolved incidents in the response, marshmallow raises a validation exception. I expected to get None values. I do not get the same exception if the incident is resolved and has a 'resolved_at' value. Am I doing something wrong?

Here is my schema:

class Incident(Schema):
    __location__ = "/api/now/table/incident"

    sys_id = fields.Text(is_primary=True)
    number = fields.Text()
    opened_at = fields.Datetime()
    caller_id = fields.Text(pluck=Joined.DISPLAY_VALUE)
    short_description = fields.Text()
    incident_state = fields.Text()
    impact = fields.Numeric()
    assignment_group = fields.Text(pluck=Joined.DISPLAY_VALUE)
    assigned_to = fields.Text(pluck=Joined.DISPLAY_VALUE)
    resolved_at = fields.Datetime()     # I tried with (pluck=Joined.VALUE) too
    resolved_by = fields.Text(pluck=Joined.DISPLAY_VALUE)

Here is the exception:

File "/..project_path_here../venv/lib/python3.8/site-packages/marshmallow/schema.py", line 904, in _do_load
    raise exc
marshmallow.exceptions.ValidationError: {0: {'resolved_at': ['Not a valid datetime.']}, 1: {'resolved_at': ['Not a valid datetime.']}, 2: {'resolved_at': ['Not a valid datetime.']}, 3: {'resolved_at': ['Not a valid datetime.']}, 4: {'resolved_at': ['Not a valid datetime.']}, 5: {'resolved_at': ['Not a valid datetime.']}, 6: {'resolved_at': ['Not a valid datetime.']}, 7: {'resolved_at': ['Not a valid datetime.']}, 8: {'resolved_at': ['Not a valid datetime.']}, 9: {'resolved_at': ['Not a valid datetime.']}, 10: {'resolved_at': ['Not a valid datetime.']}, 11: {'resolved_at': ['Not a valid datetime.']}}
sulbig commented 4 years ago

I was able to work around this issue by adding 'allow_none=True' to the 'resolved_at' Datetime() field and a new class function decorated with '@pre_load' like:

class Incident(Schema):
    __location__ = "/api/now/table/incident"

    sys_id = fields.Text(is_primary=True)
    number = fields.Text()
    opened_at = fields.Datetime()
    caller_id = fields.Text(pluck=Joined.DISPLAY_VALUE)
    short_description = fields.Text()
    incident_state = fields.Text()
    impact = fields.Numeric()
    assignment_group = fields.Text(pluck=Joined.DISPLAY_VALUE)
    assigned_to = fields.Text(pluck=Joined.DISPLAY_VALUE)
    resolved_at = fields.Datetime(allow_none=True)
    resolved_by = fields.Text(pluck=Joined.DISPLAY_VALUE)

    @pre_load
    def translate_empty_strings(self, data, **kwargs):
        data['resolved_at'] = data.get('resolved_at') or None
        return data

Is it possible that this can be implemented more generically for all Datetime() schema fields?

rbw commented 4 years ago

I've improved the response content deserialization in https://github.com/rbw/snow/pull/39, it should fix this issue. I'd appreciate if you could test it out.

Thanks for reporting.

sulbig commented 4 years ago

I pulled the 'fix-field-deserialization' branch and re-tested my scenario by removing the @pre_load and the allow_none=True argument on the Datetime() on the resolve_at field. No exceptions were raised and empty values were deserialized as None values. Non-None Datetime() values like opened_at were returned correctly, too.

rbw commented 4 years ago

Fixed in 0.2.1