TG2 Validate

1 view
Skip to first unread message

percious

unread,
Jan 23, 2008, 10:49:51 PM1/23/08
to TurboGears Trunk
I was wondering why this code has been removed. I find it very
usefull.

#TODO: Consider depricating this in favor of pylons validate decorator
class validate(object):
"""Validate regesters validator on the decorated function.

:Parameters:
validator
Valdators
error_handler
Assign error handler
"""
def __init__(self, validator=None, error_handler=None, **kw):
if not hasattr(validator, 'to_python') and hasattr(validator,
'validator'):
validator = validator.validator
elif kw:
assert validator is None, \
'validator must not be specified with additional
keyword arguments'
validator = kw
if not isinstance(validator, formencode.Schema):
validator = _schema(validator)
self.validator = validator
self.error_handler = error_handler

def __call__(self, func):
deco = TGDecoration.get_decoration(func)
deco.validator = self.validator
deco.error_handler = self.error_handler
return func


The way I use this code is to retrieve the target form from my widget
cache in the following manner:

class DBMechanicValidator(object):
@property
def validator(self):
if not pylons.request.method == 'POST':
return func(self, *args, **kwargs)
if post_only:
params = pylons.request.POST.copy()
else:
params = pylons.request.params.copy()

sprocket = self.sprockets[params['dbsprockets_id']]
form = sprocket.view.widget
return form

Now, maybe I could write a formencode.Schema that does the same thing,
but i think I need to be pointed in the right direction to get that
working.

cheers.

Mark Ramm

unread,
Jan 23, 2008, 11:29:13 PM1/23/08
to turbogea...@googlegroups.com
Hmm, I need to catch up on what happened to validate during the
sprint, and perhaps stop banging my head against a wall with 2.4
compatability issues that definitely walk the dependency chain back up
to pylons...

I think there was a notion to make validate into a "real" decorator
with no particular hooks in the TurboGears controller's __call__
method. That would make it easy enough to replace the TG validator
with custom validators from tosca, or wherever.

Max Ishenko also had some ideas on how he would like TG validators to
be better, coming from newforms I think. I think Alberto did a bit
of hacking around, but I don't think Max made any validator
changes....

I'll look into this more and see what's happening.

But I'm definitely open to ideas about how to make validation better
in TurboGears 2...

So, if there's something missing from the current version that was in
there before, feel free to add it back. ;)

And if there's something that it should be doing better, (like taking
form as a keyword param as mentioned in another thread), I'm open to
making those changes too.

--Mark Ramm

--
Mark Ramm-Christensen
email: mark at compoundthinking dot com
blog: www.compoundthinking.com/blog

percious

unread,
Jan 23, 2008, 11:41:07 PM1/23/08
to TurboGears Trunk
I think this might be a change to the pylons code base.

I am just having a hard time creating validators where the forms are
dynamic... Wonder what I would need that for?

-chris

Mark Ramm

unread,
Jan 23, 2008, 11:44:21 PM1/23/08
to turbogea...@googlegroups.com
This part of the pylons code base is very much up to us to figure out
as it was added by Jonathan, Rick and me, at Ben's request. So if
you have changes you want made just send me an hg bundle and I'll
review them and get them pushed up to Ben for inclusion into pylons.

--Mark

Jonathan LaCour

unread,
Jan 24, 2008, 8:55:02 AM1/24/08
to turbogea...@googlegroups.com
Mark Ramm wrote:

> I think there was a notion to make validate into a "real"
> decorator with no particular hooks in the TurboGears controller's
> __call__ method. That would make it easy enough to replace the TG
> validator with custom validators from tosca, or wherever.

Count me as a huge -1 on this idea. "Real" decorators are often
"real" difficult to debug. I'd greatly prefer it if we could just
make the existing validate decorator more extensible by allowing you
to pass in callables. All of the gain, with none of the hassle!

We specifically did it this way, because we didn't like how the
previous TurboGears 1.x decorators ended up nesting your controller
methods tons of times, producing insane stack traces.

--
Jonathan LaCour
http://cleverdevil.org

Mark Ramm

unread,
Jan 24, 2008, 9:37:30 AM1/24/08
to turbogea...@googlegroups.com

I agree with that on some basic level, but I also think it's important
to differentiate @validate from @expose.

@validate is designed to change the method signature, whereas @expose
should not. @validate is a reasonable candidate for somebody to
switch out with a new implementation, and @expose is not.

I'm not so worried about @validate as I was about expose, the
decorator version of @validate wouldn't require extra work to
maintaint the method signature, and would have a very well-understood
scope. I haven't done it, but I think the "actual wrapper" version
of validate wouldn't be much more code, or much more difficult to
read.

Alberto particularly wanted to be able to supply his own validate in
ToscaWidgets, and if there's signficant benifit in letting people
tweek validate, it makes sense to decouple it from the controller so
that people could much more reasonably do that.

Jonathan LaCour

unread,
Jan 24, 2008, 11:57:17 AM1/24/08
to turbogea...@googlegroups.com
Mark Ramm wrote:

> I agree with that on some basic level, but I also think it's
> important to differentiate @validate from @expose.

Okay, I sort of agree, and sort of disagree with you here...

> @validate is designed to change the method signature, whereas
> @expose should not.

This should read "@validate *was* designed to change the method
signature", but that doesn't mean that it *has* to be done this way.
Personally, I find decorators that change the method signature to
be a little bit difficult to understand, and if its hard for me to
understand, then its probably going to be really hard for beginners
to understand.

I believe that we can get all the functionality that we need in
@validate, without having to do crazy things, or create nutty stack
traces.

> @validate is a reasonable candidate for somebody to switch out
> with a new implementation, and @expose is not.

I agree with you here, but there are two ways we could do this that
are far less insane than creating a "wrapping" decorator:

1. Allow our @validate decorator to take in callables to perform
generic validation, and error handling code that someone
defines on their own. This way, people can do whatever they
like.

2. If someone wants to replace the implementation of @validate,
then they should create their own decorator, not monkeypatch
TurboGears to use their implementation.

So, I don't see this as a good argument, personally.

> I'm not so worried about @validate as I was about expose,
> the decorator version of @validate wouldn't require extra
> work to maintaint the method signature, and would have a very
> well-understood scope. I haven't done it, but I think the "actual
> wrapper" version of validate wouldn't be much more code, or much
> more difficult to read.

It'll still mangle stack traces, though, which is a big deal to
me. Also, decorators that perform real "wrapping" instantly become
significantly more complex, as they have to make sure to "migrate"
all of the function attributes from the old function to the newly
wrapped function.

> Alberto particularly wanted to be able to supply his own validate
> in ToscaWidgets, and if there's signficant benifit in letting
> people tweek validate, it makes sense to decouple it from the
> controller so that people could much more reasonably do that.

I can certainly understand that, but again, I think we can
accomplish this without having to sacrifice the elegance of the
current solution, or making the decorator more complex than it needs
to be. I'd rather see us design @validate to be extensible by
passing in callables.

Just my two cents...

Mark Ramm

unread,
Jan 24, 2008, 8:26:52 PM1/24/08
to turbogea...@googlegroups.com
> > Alberto particularly wanted to be able to supply his own validate
> > in ToscaWidgets, and if there's signficant benefit in letting

> > people tweek validate, it makes sense to decouple it from the
> > controller so that people could much more reasonably do that.
>
> I can certainly understand that, but again, I think we can
> accomplish this without having to sacrifice the elegance of the
> current solution, or making the decorator more complex than it needs
> to be. I'd rather see us design @validate to be extensible by
> passing in callables.

Well, I think the stack trace is a bit confused by all the
inspect_cal, perform_call, stuff in the controller anyway, but I
pretty much agree that if both validate and expose maintain the same
form there is a nice symmetry to that. Plus I agree that the current
validation setup is easy to understand, and I hope adding a callable
and the specific hook toscawidget hook where you can pass a form (or
anything with a .validate method) to the @validate, and possibly the
suggested ability to pass a generic callable in, should provide enough
flexibility.

But I think we should wait for Alberto to make his argument before we
make a final decision. But in the meantime I'm adding tickets for
improving the current validation system.

--Mark Ramm

percious

unread,
Jan 25, 2008, 12:07:12 AM1/25/08
to TurboGears Trunk
Ok, I am really close to having a solution that should make everyone
happy. It involves the following change in pylons:

diff -r 28eed5438061 pylons/decorators/expose.py
--- a/pylons/decorators/expose.py Sun Jan 13 17:29:05 2008 -0800
+++ b/pylons/decorators/expose.py Thu Jan 24 21:46:28 2008 -0700
@@ -203,6 +203,9 @@ class validate(object):
self.validators = validators
self.error_handler = error_handler

+ def _before_validate(self, controller, params):
+ pass
+
def __call__(self, func):
deco = Decoration.get_decoration(func)
deco.validation = self

and the following change to Turbogears:
Index: tg/decorated.py
===================================================================
--- tg/decorated.py (revision 4039)
+++ tg/decorated.py (working copy)
@@ -21,9 +21,12 @@
class DecoratedController(WSGIController):

def _perform_validate(self, controller, params):
+ print 'decorated performvalidate', controller, params
validation = getattr(controller.decoration, 'validation',
None)
+ print '&'*80, validation
if validation is None:
return params
+ validation._before_validate(controller, params)

new_params=params
if isinstance(validation.validators, dict):
@@ -103,7 +106,7 @@
namespace=namespace)
return result

- def _handle_validation_errors(self, controller, exception):
+ def _handle_validation_errors(self, controller, exception, args):
pylons.c.form_errors = exception.error_dict
pylons.c.form_values = exception.value

@@ -111,7 +114,8 @@
if error_handler is None:
error_handler = controller

- output = error_handler(controller.im_self)
+ print args
+ output = error_handler(controller.im_self, args)

return error_handler, output

@@ -140,7 +144,7 @@

except formencode.api.Invalid, inv:
controller, output =
self._handle_validation_errors(controller,
- inv)
+ inv,
args)

# Render template
controller.decoration.run_hooks('before_render', remainder,
params,


What this does is allow a hook for the validator to do whatever you
want. You just extend validate and implement _before_validate() and
set self.validators to what you actually want for a validator. It's
convenient for me to use the params to find the form....

Anyway it *almost* works, the only problem I am seeing is that when
the error_handler is called, the args from the previous calls are not
getting passed through. If anyone has an idea on how this might be
done I think this would be a really nice implementation.

cheers.
-chris

Alberto Valverde

unread,
Jan 25, 2008, 2:27:46 AM1/25/08
to turbogea...@googlegroups.com

I'm not so strong anymore on the points I expressed back then in
http://trac.turbogears.org/ticket/1426, I still favor a "real" decorator
approach because I find it cleaner (all validation done in the same
closure opposed to needing support infrastructure in the controller
subclass) and more pythonic (hey, decorators even got a "@" syntax in
2.4! ;)

As long as the validation system is pluggable I don't really care how
its implemented. The current duck-typing for a validate() method on the
validator object sounds like solution, maybe even better passing the
validation function directly (ie: "validate"), but needs a defined
protocol to handle validation errors since raising a FormEncode Invalid
might get in Max's way when he tries to adapt it to use django's
newforms. Maybe just returning True if validation passes or a
"validation errors" object that evaluates to False when it doesn't?

Alberto

percious

unread,
Jan 25, 2008, 9:40:06 PM1/25/08
to TurboGears Trunk
Ok, I am pretty sure I got it all working. The call to error_handler
was missing the args sent into the function for some reason, so I
passed them through. This seems to work fine.

Before I commit these changes, would someone mind making the change to
pylons that I described above?

Thanks,
chris

On Jan 25, 12:27 am, Alberto Valverde <albe...@toscat.net> wrote:
> Mark Ramm wrote:
> >>> Alberto particularly wanted to be able to supply his own validate
> >>> in ToscaWidgets, and if there's signficant benefit in letting
> >>> people tweek validate, it makes sense to decouple it from the
> >>> controller so that people could much more reasonably do that.
> >> I can certainly understand that, but again, I think we can
> >> accomplish this without having to sacrifice the elegance of the
> >> current solution, or making the decorator more complex than it needs
> >> to be. I'd rather see us design @validate to be extensible by
> >> passing in callables.
>
> > Well, I think the stack trace is a bit confused by all the
> > inspect_cal, perform_call, stuff in the controller anyway, but I
> > pretty much agree that if both validate and expose maintain the same
> > form there is a nice symmetry to that. Plus I agree that the current
> > validation setup is easy to understand, and I hope adding a callable
> > and the specific hook toscawidget hook where you can pass a form (or
> > anything with a .validate method) to the @validate, and possibly the
> > suggested ability to pass a generic callable in, should provide enough
> > flexibility.
>
> > But I think we should wait for Alberto to make his argument before we
> > make a final decision. But in the meantime I'm adding tickets for
> > improving the current validation system.
>
> I'm not so strong anymore on the points I expressed back then inhttp://trac.turbogears.org/ticket/1426, I still favor a "real" decorator

percious

unread,
Jan 25, 2008, 10:46:14 PM1/25/08
to TurboGears Trunk
Ok, I committed this valuable change, the change I made will not
require a change to pylons, but if we do change pylons, we can
eliminate one if statement in the validator.
cheers.
-chris

iain duncan

unread,
Jan 25, 2008, 10:57:58 PM1/25/08
to turbogea...@googlegroups.com
On Fri, 2008-25-01 at 19:46 -0800, percious wrote:
> Ok, I committed this valuable change, the change I made will not
> require a change to pylons, but if we do change pylons, we can
> eliminate one if statement in the validator.

That sounds like a reasonable amount of overhead! ;-)

Thanks for all your work Chris.
Iain


percious

unread,
Jan 26, 2008, 12:01:04 AM1/26/08
to TurboGears Trunk
Just look at this sweetness:

from tg import validate

class DBMechanicValidate(validate):

def _before_validate(self, controller, params):
sprocket =
controller.im_self.sprockets[params['dbsprockets_id']]
form = sprocket.view.widget
self.validators = form

Makes me smile just to think about it.

Mark Ramm

unread,
Jan 29, 2008, 7:29:30 PM1/29/08
to turbogea...@googlegroups.com
On Jan 25, 2008 9:40 PM, percious <ch...@percious.com> wrote:
>
> Ok, I am pretty sure I got it all working. The call to error_handler
> was missing the args sent into the function for some reason, so I
> passed them through. This seems to work fine.
>
> Before I commit these changes, would someone mind making the change to
> pylons that I described above?

My current pylons is broken due to lots of 2.4 webob compatability
fixes, but I think I'll archive this branch for posterity and just
accept the 2.5 requirement for a bit so, I should be able to get this
merged into my pylons branch, and pushed back up to ben sometime this
week.

At work we have a python 2.4 requirement, so I'm finding that I'm not
able to progress on tg2 as quickly as I'd like. And at the same time
we have a big project due in the next couple of weeks, so Unfotunately
my TG2 time is going to continute to be reduced for a little bit.

Reply all
Reply to author
Forward
0 new messages