Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

include the requirement flag on the thrift spec #72

Merged
merged 1 commit into from
Jan 18, 2015
Merged

include the requirement flag on the thrift spec #72

merged 1 commit into from
Jan 18, 2015

Conversation

tmehlinger
Copy link
Contributor

This amends the parser to include the requirement flag so it can be used for later validation.

I've used this in my own implementation of TProcessor (not included in these changes) to accomplish something like this:

struct Thing {
    1: required string required_field;
}
class ValidatingProcessor(TProcessor):
    def validate(self, result):
        obj = result.success
        for _, s in obj.thrift_spec.items():
            k = s[1]
            r = s[-1] # requirement will be at the end of the tuple
            v = getattr(obj, k)
            if v is None and r:
                raise TApplicationException(
                        message='{} is required'.format(k))

    def process(self, iprot, oprot):
        api, seqid, result, call = self.process_in(iprot)
        if isinstance(result, TApplicationException):
            self.send_exception(oprot, api, result, seqid)
            return
        try:
            result.success = call()
            self.validate(result)
        except TApplicationException as e:
            self.send_exception(oprot, api, e, seqid)
        except Exception as e:
            # raise if api don't have throws
            self.handle_exception(e, result)
        else:
            self.send_result(oprot, api, result, seqid)

Which, when called, produces:

$ python failing_client.py
<backtrace snip>
thriftpy.thrift.TApplicationException: `required_field` is required

@tmehlinger
Copy link
Contributor Author

Tests are passing but this doesn't work right with complex (map, list, etc.) types. Please don't merge.

* Updated tuple size checks and unpacking to handle introduction of the
  requirement flag.
* Updated tests.
@tmehlinger
Copy link
Contributor Author

Should be fixed now.

lxyu added a commit that referenced this pull request Jan 18, 2015
include the requirement flag on the thrift spec
@lxyu lxyu merged commit 20de048 into Thriftpy:develop Jan 18, 2015
@hit9
Copy link
Contributor

hit9 commented Jan 24, 2015

Parser has been rebuit, but this feature was included.

@tmehlinger tmehlinger deleted the include-requirement branch March 25, 2015 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants