Hi Chris,
On Tue, Jun 11, 2013 at 4:29 PM, Christopher Medrela <
chris.medr...@gmail.com> wrote:
> Hello!
>
> **Making distinction between form validation and static validation.** I
> named
> the process of static checks of apps, models etc. "validation".
> Unfortunately,
> novices may confuse it with form validation. So I propose to rename it to
> "verification". Functions/methods/classmethods (referred to as just
> functions)
> starting with `validate_` will start with `verify_` now. And functions
> discovering all other verification functions will be called `verify`
> instead
> of `validate_all`. This is shorter than `validate_` (only 6 letters) and
> there
> is no risk of confusing novices. I considered alternatives: "check" is too
> general and "static_check" as well as "static_validate" are too long.
>
I can't say I'm completely in love with 'verify' as a name, but I can't say
I've got any better options either. We definitely need to get away from
"validate". I agree that "check" is a bit too generic, and appending
something to 'check' is only going to make it unwieldy (although there's a
certain beauty in having a generic name like 'check' for a generic
'checking' feature)
Lets go with verify for the moment; if the process of building this whole
framework reveals another workable name, we'll be a search-and-replace away
from a fix.
> **Disabling/enabling parts of verification**
>
> > Hi Christopher,
> >
> > Overall, this looks like a great project and I look forward to the more
> > flexible validation hooks. As the author of django-secure, I do have one
> > concern with the implementation plan:
> >
> > Django's existing validation checks are intended to be run in
> > development, as a debugging aid, but not in production (to avoid any
> > slowdown). Thus, by default they are run prior to many management
> > commands (such as syncdb, runserver), but are generally not run on
> > production deployments.
> >
> > The checks in django-secure, on the other hand, are intended to be run
> > only under your production configuration, and are mostly useless/wrong
> > in development. Since runserver doesn't handle HTTPS, most people don't
> > use HTTPS in development, which means you can't set e.g.
> > SESSION_COOKIE_SECURE to True in development or your sessions will
> > break, which means that django-secure check will fail; same goes for
> > most of the other checks. Running django-secure's checks every time you
> > syncdb or runserver in development would be at best a waste of time and
> > at worst extremely annoying.
> >
> > So I think the validation framework you build has to incorporate some
> > kind of distinction between these two types of validation, which have
> > very different purposes and should be run at different times:
> > development/debugging validation and production-configuration
> > validation. I'm not sure exactly what form this distinction should take:
> > perhaps validators are simply tagged as one type or the other and then
> > there are two different management commands?
> >
> > Interested in your thoughts,
> >
> > Carl
>
> DEBUG setting is a hint about the environment; if you are working in
> development environment, then it's likely that DEBUG is set to True; and
> you
> have DEBUG set to False in production, I hope. By default, `verify_*`
> functions will be run only when DEBUG is set to True. django-secure checks
> will be decorated with new `run_only_in_production` decorator and will run
> only when DEBUG is set to False.
>
> However, I'm not sure if this is the best solution. There are some
> disadvantages, i. e. I can imagine people saying "what to do if I want to
> run
> security checks while DEBUG is set True?". Of course, they can set DEBUG to
> False, but it's only a workaround. I afraid that it's only tip of the
> iceberg
> -- we will probably need more control over which checks to run.
>
> An another solution, much more flexible, is to pass around a "filter
> function"
> which takes a `verify_*` function and says if the function should be
> called. A
> drawback is that this forces developers to add an argument to every
> `verify_*`
> function even though they don't want it. In order to avoid that we can
> introspect the function and check if the function requires `filter`
> argument
> and in that case call it without any argument, otherwise call it passing
> the
> filter function as `filter` argument, but this is hacky, ugly and
> non-pythonic; and if anybody wants to rewrite `verify` function (which
> collect
> all `verify_` functions and run them), they need to know about that trick
> and
> how to use it.
>
> So we can split `verify_` functions into two groups: one which does the
> checks
> and doesn't require the filter function and another one which finds all
> `verify_` functions and run them. An example of the former is
> `verify_upload_to_attribute` of `FieldFile`; and an example of the latter
> is
> `verify` function. The latter requires the filter function as an argument.
> We
> need to come out with name pattern for the former (I have no idea now, I
> will
> propose something tomorrow). This is the most flexible solution, but it
> increases complexity.
>
> I prefer the last, most flexible solution.
>
This is starting to sounds a bit like YAGNI (You Aint Gonna Need It) to me.
I can see the flexibility that you're aiming to introduce, but frankly, I'm
having difficulty thinking of a real world situation where this flexibility
would actually need to be used.
There's an obvious "am I in production?" difference that needs to be
accounted for, but I think the DEBUG=True/False should be enough to cover
that. There's an edge case where I need to turn DEBUG off in development to
test some particular feature, so there may be a need to have an option on
the 'verify' command to ignore errors, or at least 'don't stop when you see
an error'. The Django-secure approach of settings to silence specific
errors may also be an approach here; if there's going to be a lot of these
errors, collapsing 'acceptable errors' into a single setting might be an
idea (analogous to the configuration of PyLint)
Adding a whole dynamic filtering framework for verification checks seems
excessive to me unless you've got a specific use case in mind.
If we really want to protect ourselves from the future, we can take the
same approach that model and form save() methods use -- define the official
prototype to be verify(**kwargs). That way we retain the flexibility to
introduce flags at some later date
> **Breaking points.** As Russell suggested me during private conversation,
> I will
> try to merge my branch as often as possible. That should increase chance of
> success. There is one obvious breaking points: after finishing revamping
> validation.
>
> Merging while working on the framework shouldn't be too hard. That is
> because
> I will refactor bottom-up and keep all code and tests running and because I
> won't touch any public API. Actually, merging can be done after every week.
>
> First question: Django 1.6 is being released now and there is no 1.7
> branch yet; will be there any 1.7 branch where I can merge to?
>
> We'll fork the Django 1.6 release branch once the beta is released (in
theory, at some point in the next couple of weeks). At that point the
Django 1.6 branch will be frozen for new features, and the 1.7 branch will
be where new features land.
> Another issue is if we can merge a branch when documentation is not
> finished.
> I will create a new doc page at 3rd week and I will be completing it to the
> end of 8th week. I guess that it's not allowed to merge code with partial
> doc.
> A solution may be to merge code without doc and merge the doc only at the
> end.
>
> My schedule didn't change, but I will paste it here:
>
> 1. Rewriting tests of field validation (1 week)
> 2. Rewriting field validation (1 week)
> 3. Writing documentation (mainly overview) (0.5 week)
> 4. Tests, implementation and documentation of models validation (1
> week)
> 5. Tests, implementation and documentation of apps validation (1 week)
> 6. Rewriting validation of custom User model (0.5 week)
> 7. Tests, implementation and documentation of manager validation (0.5
> week)
> 8. Rewriting validation of AdminModel and its tests (1 week)
> 9. Rewriting mechanism of triggering validation framework (1 week)
> 10. Finishing documentation (0.5 week)
>
> I would like to make the first merging after the first week (rewriting
> tests of
> field validation), because it will make me familiar with the process of
> merging and will make next merging (which will be much bigger) easier. The
> first week is only refactoring, no doc, no new features, so I guess it can
> be
> merged.
>
> If doc is not a problem then we can merge after 3. task (field validation
> implemented) and after 6. task (models and app validation implemented).
> And of
> course, in the end.
>
> If it comes to the second part of my project (merging django-secure), it's
> obvious there is only one merging point. :)
>
Merging incomplete features isn't an option, and having incomplete
documentation counts as an incomplete feature.
I'm also wary of the fact that we may need to change some architectural
features during the development process. I was interested in the interim
merge approach because having that granularity can help focus development
effort onto deliverable chunks; however, it sounds like it will be
difficult to take this approach for your project. We can assess this as we
go, but lets work on the assumption that we're not going to do a
mid-project merge.
> **Some changes to public API.** On last Friday (7 June 2013) I gave a
> small talk
> about my project at pykonik (a meeting of Python programmers in Kraków
> where I
> live). I was given some feedback.
>
> I changed fields of Error and Warning classes. Previously I proposed
> splitting
> error message into `msg` (short error message), `explanation` (why is some
> situation disallowed?) and `hint` (list of solutions). We agreed that
> `explanation` is redundant, so now I propose only two fields: `msg` and
> `hint`. `hint` will be optional, so you can set it to None, but you have
> to do
> it explicitly:
>
> Error("something is broken", hint=None)
>
> I hope that this will force developers to thing carefully if they cannot
> give
> any useful hint and that it will improve quality of error messages.
>
I agree that having a message *and* a reason is overkill. I also like the
idea of having an explicit API that encourages developers to provide a
hint. My question would be whether that hint should be a string, or a list.
If it's a list, you're encouraging the developer to think about multiple
options for fixing the problem (e.g., rename the field *or* add a
related_name), and having that structure in the error message itself gives
you some flexibility in how to display the hints (e.g., you can print them
one per line).
One extra thing we may need to add is an error identifier. If we're going
to have a generic PyLint-like method to silence specific errors/warnings,
it's important that we have a way to easily identify specific error types.
PyLint assigns each rule an identifier (e.g., E121 - "Continuation line
indentation is not a multiple of 4"), and if you decide that rule isn't
important, you can put "E121" in your error exclusion list.
**django-security** During the meeting, somebody referred to
> django-security [1],
> and I will have a look at it soon. Implementing some features of
> django-security
> may be a good idea if I finish merging django-secure earlier.
>
> [1] https://github.com/sdelements/django-security
>
I'm not familiar with django-security either; from a quick inspection, it
seems to be more about security related features (such as password expiry)
rather than security validation checks. Lets leave this on the todo pile
for the moment. If we get to the end of the project and by some miracle
we're ahead of schedule, we can look at this again. I'm sure we won't have
a shortage of interesting things we could add as features.
Yours,
Russ Magee %-)
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
