- changed status to duplicate
Validator methods are not called for synonyms and inherited columns
Consider following example of joined-table inheritance:
class Parent(DeclarativeBase):
# ...
parent_column = Column(String(255))
class Child(Parent):
# ...
child_column = synonym('parent_column')
@validates('child_column') # @validates('parent_column') also does not work
def validator(self, name, value):
# not called at all
raise ValueError()
The validator
method is never called
Comments (13)
-
repo owner -
repo owner validate against "parent_column", not the synonym. the patch in 2943 breaks backwards compatibility for apps that may rely upon a validator silently failing so this is targeted at 1.0 for the moment.
-
repo owner also you can see there is some decision to be made if both validators should run, or only one. I really have no idea what ppl would prefer.
-
reporter @zzzeek sorry for dup; IMO I think that both validators should run, and probably validator for synonyms also should be possible, this could form a nice validation chain, from specialist models through base ones. Maybe a default-false mapper attribute to turn on "new behaviour"?
-
repo owner OK which validator runs first ?
-
repo owner if only the "sub" validator runs, and the "super" runs by calling it explicitly, then the user can control it.
-
reporter The most specific one should run first, and still I think every one should run, without exceptions, object should be valid from any object perspective, base one, and "child" one.
-
repo owner what if I want to control the order in which they run? wouldn't using super() be more pythonic?
-
repo owner e.g.
class B(A): @validates('data', override=True) def validate(self, key, value): return "yup B " + super(B, self).validate(key, value)
-
repo owner synonyms is a whole can of worms. does the synonym-bound validator run only if one accesses the attribute via the synonym name, or by either it's syn name + real name? how about for chained scenarios? this is very complicated to implement as synonyms aren't resolved until late. for now im just going to have that raise.
-
reporter With big power ('configurability') comes big responsibility, IMO this should be kept simple, like a convention: From base model point of view any sub-model provided value should be valid input (go through base model validation nicely), child classes should only make more specific validations. I think we should not over-engineer that.
What if base model validation assumes that string column should be 'numeric' would you allow child model to pass non numeric input, and in reality make base model "invalid" from its point of view? This sounds wrong from the first place for me.
But this is only my opinion, others may disagree.
-
repo owner on #2943 I've proposed 'override' can be set to 'add' or 'replace'. this isn't an excessive amount of engineering, it's just another line of code, and allows both versions to take place thus releasing me of being hit with a use case we aren't thinking of at the moment.
-
reporter If it's not a hard to implement then it's ok. I'll try to design some unit tests for yours patch, but I'm not familiar with sqla code, so it may take a while. Now good night, it's almost 3am here in London ;)
- Log in to comment
Duplicate of #2943.