sanic-wtf

sanic-wtf copied to clipboard

#13

New features:

• Support for async and sync validators
• testing py37

The code I added isn't of best quality, especially the tests. This is only meant to be a prototype.

I think in order to merge this feature, we'd have to go 1 of the following paths:

1. Break backward compatibility and make validate_on_submit a coroutine.
2. Keep both validate_on_submit and validate_on_submit_async (maybe name it differently). However, if we do this, then the user shouldn't call validate_on_submit_async followed by validate_on_submit, because validate_on_submit_async will do lots of monkey patching (We can have some sort of flag though that explicitly enables this functionality).
3. Make a new SanicForm class. Maybe call it SanicForm_ or sth
4. Do nothing :smile:

So, I did some cleanup. I think the only thing missing is patching FieldList. If left as is, then it will only be called synchronously by any given form. However, it will fail if any of it's methods are coroutines as they will not be awaited. Basically, as long as you don't use custom FieldList with coroutines in it, you're fine.

As for the tests coverage, I think I covered the main use cases. There might be some untested edge cases though, so if someone can help me spot some, it would be great.

Thanks for the feature, I was very busy lately, I did not have time to go through the code right now, do you mind give me sometime. Thanks again.

Sure, np! :)

Hi Omar, sorry that I was super busy lately, it seems you put a lot of effort into this, I will go through this in a few days.

Thank you so much.

Hi Omar Ryhan I am sorry I failed in manage time to update the project, are you still interested in using this? If you are interested, I will put you into collaborators list.

@mikekeda How about this one, @omarryhan invested a lot of time into this, what's your thought?

@mikekeda I at OP accidentally in last post.

Hey @pyx. No worries.

I only passively maintain the Sanic projects I was working on. The Sanic team recently introduced some breaking changes to the request context API and I'm not really sure whether my PR might break things or not. My Sanic projects are still running on their latest LTS version which differs from their latest version. I'm pip installing from my own sanic-wtf repo, and things are working fine.

I suggest we leave this PR open until someone shows some interest and maybe tests things out on the latest Sanic version, then maybe we can merge it?

I must also add that I monkey patched some of wtform's private methods to make this lib support async validations. The downside of this that wtforms cannot guarantee us that they won't break their private APIs. So I hard-coded the wtforms version to avoid things breaking. One downside of that is that we'll not be getting the latest security patches from wtforms should there be any.

@omarryhan you are right, relying on private API is fragile to say the least and we should try to avoid. We will wait and see.

Please remember to add your name in AUTHOR with next commit, not necessarily this one but can be something like a separate one updating python versions tested upon, you have the commit rights. Thank you.

@mikekeda How about this one, @omarryhan invested a lot of time into this, what's your thought?

1. I think it's better to split this into 2 RPs Async validators and Recaptcha
2. monkey patching doesn't look good to me, can we create a separate class for this?
3. What performance improvement we will get with Async validators?
4. I don't think it's a good idea to make RECAPTCHA required, it's better to make it optional (in RECAPTCHA_PUBLIC_KEY and RECAPTCHA_PRIVATE_KEY are marked as required)
5. Why clear and clear_pycache.sh are committed?

Thanks for taking the time to review @mikekeda

I added async not for the performance gain but to be able to call async code for validations, like querying a database to check if an email exists.

I'll address these issues once people show enough interest in the PR. Right now, it seems like I'm the only one using it.