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

struct required field is not verified #119

Open
shuoli84 opened this issue Feb 20, 2020 · 4 comments
Open

struct required field is not verified #119

shuoli84 opened this issue Feb 20, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@shuoli84
Copy link

Similar as #72 , required field in struct should be enforced before send out. Current implementation only validate method args.

Since the init func is generated on the fly, so maybe we should add some validation code in init?

    """Generate `__init__` function based on TPayload.default_spec

    For example::

        spec = [('name', 'Alice'), ('number', None)]

    will generate a types.FunctionType object representing::

        def __init__(self, name='Alice', number=None):
            self.name = name
            self.number = number

            if number is None: raise ValueError()

This way actually breaks init then assign behavior. Or add a validate method on each generated class and call it just before send?

@ethe
Copy link
Member

ethe commented Feb 23, 2020

Yes, I think Thriftpy should validate the arguments are all passed, even it breaks the compatibility.

@ethe ethe added the enhancement New feature or request label Feb 23, 2020
@iamsudip
Copy link
Contributor

@ethe I have some free time these days. Since I worked on enforcing required args issue, I guess somewhat I have some idea. Can you point me how to implement this?

@ethe
Copy link
Member

ethe commented May 28, 2020

@iamsudip Thank you, I think we can modify the function thrift.py:init_func_generator, and change all required field from keyword arguments to normal arguments, and please make sure that the argument sequence is matched with thrift IDL.

iamsudip added a commit to iamsudip/thriftpy2 that referenced this issue May 28, 2020
iamsudip added a commit to iamsudip/thriftpy2 that referenced this issue May 28, 2020
@iamsudip
Copy link
Contributor

iamsudip commented May 28, 2020

@ethe Can you have a look at #132

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants