Re: [GSoC] Revamping validation framework and merging django-secure once again
In order to get more attention, I've created a new thread [1]. [1] https://groups.google.com/forum/#!topic/django-developers/_sde4eoNRyQ -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On Sun, 2013-09-15 at 18:23 +0200, Aymeric Augustin wrote: > On 15 sept. 2013, at 17:57, Simon Kern wrote: > > > Yes but management commands should be irrelevant for django-secure > > Well, in this case, I have a backup argument :) > > There's a non-negligible number of people serving websites in production with > ./manage.py runserver, in spite of all the warnings in the docs. FWIW there is also a non-negligible number of people who believe in doing development using the same stack that they're going to deploy to for production use, and who therefore never use the runserver at all... Cheers, Nick -- Nick Phillips / nick.phill...@otago.ac.nz / 03 479 4195 # These statements are mine, not those of the University of Otago -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On 15 sept. 2013, at 20:07, Michael Manfre wrote: > No amount of code or docs will fix all of the stupid things people do. > Of course, but that isn't a sufficient reason for disabling the security checks. The point of django-secure is to help users with limited knowledge of security best practices. That includes people using runserver in prod. I'm not sure if you were making a counter-argument or just a tangential comment. Regardless, DEBUG is the canonical way to distinguish dev from prod in Django. Code based on another assumption won't be committed. There's a long term goal to have a more versatile concept of "environment" but that's another discussion. If you want to have that discussion now, please move it to another thread :) -- Aymeric. -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On Sep 15, 2013 12:23 PM, "Aymeric Augustin" < aymeric.augus...@polytechnique.org> wrote: > > On 15 sept. 2013, at 17:57, Simon Kern wrote: > > > Yes but management commands should be irrelevant for django-secure > > Well, in this case, I have a backup argument :) > > There's a non-negligible number of people serving websites in production with ./manage.py runserver, in spite of all the warnings in the docs. > No amount of code or docs will fix all of the stupid things people do. -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On 15 sept. 2013, at 17:57, Simon Kern wrote: > Yes but management commands should be irrelevant for django-secure Well, in this case, I have a backup argument :) There's a non-negligible number of people serving websites in production with ./manage.py runserver, in spite of all the warnings in the docs. -- Aymeric. -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Yes but management commands should be irrelevant for django-secure Am 15.09.13 17:52, schrieb Aymeric Augustin: > On 15 sept. 2013, at 16:40, Simon K. wrote: > >> But in production the entry point is the wsgi.py file, isn't it? > It's the main entry point in production, but not the only one; manage.py / > django-admin.py is still used to run management commands. > -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On 15 sept. 2013, at 16:40, Simon K. wrote: > But in production the entry point is the wsgi.py file, isn't it? It's the main entry point in production, but not the only one; manage.py / django-admin.py is still used to run management commands. -- Aymeric. -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Am Mittwoch, 17. Juli 2013 10:20:47 UTC+2 schrieb Russell Keith-Magee: > > > On Mon, Jul 15, 2013 at 8:17 PM, Christopher Medrela > > > wrote: > >> Progress: I've implemented manager checks. >> >> This API allows us to register, among other things, app-specific checks. >> But >> it's not necessary to write an app in order to do some checks. >> >> `register` function will have two optional parameters: `run_in_production` >> (default: False) and `run_in_development` (default: True) that let you >> specify >> when the callable should be called. Most checks should be performed only >> in >> development environment (which is equivalent to DEBUG==True). Security >> checks >> makes sense only in production environment (<==> DEBUG==False). >> >> I would be happy if I could hear your opinion! >> > > Hi everyone, just out of curiosity (excuse me if I am wrong): Isn't there already a strong determing factor of whether a django project is running "in development" or "in production" despite the debug setting? In development you would usually use manage.py to invoke your devserver. But in production the entry point is the wsgi.py file, isn't it? At least this seems to be a more reliable approach for me than the pure differentiation based on the state of the debug setting. Simon -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
> > 4. More important changes in code: >> >> - Introduced RAW_SETTINGS_MODULE [1]. I use it in compatibility checks to >>> test >> >> if `TEST_RUNNER` setting was overriden [2]. >> >> > > Have a look at the internals of the diffsettings management command -- I'm >> not sure RAW_SETTINGS_MODULE is needed. > > > The problem is that when a test is decorated by `override_settings`, we > need to test `settings._wrapped._wrapped`, otherwise we need to check `settings._wrapped`. I and Russell agreed that this is not a huge problem since we can traverse through all "layers" of settings wrappers. However, there is another problem. By default, there are two settings file for tests: `django/conf/global_settings.py` and `tests/test_sqlite.py`. Settings from both files are mixed and indistinguishable. When no `override_settings` decorator is in use, there is only one 'layer' of settings. That means that we cannot test if `TEST_RUNNER` was overriden in `test_sqlite.py`. Any solutions? I've just noticed that we need to implement `__str__` method for ModelAdmin > *class* (not its instances) so errors involving ModelAdmins are printed > correctly. I will work at that. We decided to use "applabel.ModelAdminClassName: (admin.E033) error message" style for ModelAdmin issues, so its `__str__` method should return "applabel.ModelAdminClassName". However, ModelAdmin class doesn't know which app defines it. That means it's impossible to implement its `__str__` method. Proposals: (1) Use ModelAdmin instances instead of the class as `obj` parameter to `Error` constructor. The drawback is that we need to pass also `site`. (2) Refactoring. Now ModelAdmin class does not know anything about model it references to (is it a design decision?). After refactoring, ModelAdmin will have the model attached to itself. (3) Hardcode that case in `CheckMessage.__str__` [1]. All three doesn't look like a good solution. [1] https://github.com/chrismedrela/django/blob/c010cd06619503b7d0db914c04c6ed58c69a9d9c/django/core/checks/messages.py#L35 We probably cannot move checks of `primary_key` and `unique` living in >> `FileField.__init__`. We test if one of these two parameters was passed; >> we >> don't check their values. Consider that an user passes unique=False. This >> is >> the default value, but nevertheless, this should result in an error. We >> cannot check if the attributes where passed while performing system >> checks. I'm >> not sure if I make myself clear. > > > Why not just store a reference to the original arguments (or the relevant > subsets) in __init__(), and then validate them later in a system check? > That may seem a little indirect, but I think the validation system will be > much nicer if we can do everything in system checks instead of splitting > the work with __init__(). That's a good idea. Done in this commit [2]. [2] https://github.com/chrismedrela/django/commit/03fe680854b863020d865af5041172e4bd49943e -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
> > > We probably cannot move checks of `primary_key` and `unique` living in > `FileField.__init__`. We test if one of these two parameters was passed; we > don't check their values. Consider that an user passes unique=False. This > is > the default value, but nevertheless, this should result in an error. We > cannot check if the attributes where passed while performing system > checks. I'm > not sure if I make myself clear. > > Why not just store a reference to the original arguments (or the relevant subsets) in __init__(), and then validate them later in a system check? That may seem a little indirect, but I think the validation system will be much nicer if we can do everything in system checks instead of splitting the work with __init__(). I think there are other places as well where we'd like to do this kind of check. For example, right now you can create a OneToOneField with unique=False and Django will silently overwrite that value and your model will validate just fine. It would be much better if trying to specify unique on a OneToOneField caused a validation error, and the same sort of pattern could be applied there: storing a reference to the original value during __init__(), overwriting with unique=True for the purposes of initializing the parent ForeignKey, and then checking the user-supplied value in a separate system check. (This is just an example, I'm not saying that you should make this change to OneToOneField now, as it's backwards-incompatible.) -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
I've written four more tests. I've moved some of checks out of `__init__` methods. I've changed `tag` to `register`. I've rebased against master. I've improved documentation (added sections about filtering and silencing system checks), but I'm still polishing it. Code is ready for final review except for the issues I will mention in this post. We probably cannot move checks of `primary_key` and `unique` living in `FileField.__init__`. We test if one of these two parameters was passed; we don't check their values. Consider that an user passes unique=False. This is the default value, but nevertheless, this should result in an error. We cannot check if the attributes where passed while performing system checks. I'm not sure if I make myself clear. We must reject fk/m2m to neither a model nor model name in `__init__`. Some pieces of code (i.e. `RelatedField.contibute_to_class`) assume that `self.to` is a Model or a string. 4. More important changes in code: >> - Introduced RAW_SETTINGS_MODULE [1]. I use it in compatibility checks to >> test >> if `TEST_RUNNER` setting was overriden [2]. > > Have a look at the internals of the diffsettings management command -- I'm > not sure RAW_SETTINGS_MODULE is needed. The problem is that when a test is decorated by `override_settings`, we need to test `settings._wrapped._wrapped`, otherwise we need to check `settings._wrapped`. 2. I'm afraid that there is too little time to merge django-secure. >> September 16 is >> suggested 'pencils down' date, so there are only less than two weeks (12 >> days) + >> one buffer week, because firm 'pencils down' date is September 23. Merging >> django-secure in such little time is actually impossible -- we must >> through out >> this task from schedule. I really apologize for that. > > This is obviously disappointing, but it's better to deliver something > complete than a half-attempt. If we can get the core framework into a good > state, merging django-secure is a self-contained task that we can address > as a follow-up commit. > Also -- the GSoC will come to an end, but that doesn't mean your > contributions to Django have to… :-) Sure. However, after merging I would like to focus on other kinds of contributing like reviewing tickets, replying on mailing list and so on) -- it's easy to get bored if you do the same job all the time. - Introduced `BaseAppCache.get_models_from_apps` method [3]. This method >> returns >> all models of given set of apps (or of all apps if None is passed) and >> is used >> in `default_checks.py` [4]. > > I'm not sure I follow why this is needed -- or why it isn't just > duplicating functionality from loading.get_models()? > `get_models(app=someapp)` let you get all models from *one* app. I notice that I sometimes need to get all models from a *set* of apps. I've just noticed that we need to implement `__str__` method for ModelAdmin *class* (not its instances) so errors involving ModelAdmins are printed correctly. I will work at that. -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Hi Christopher, On Tue, Sep 3, 2013 at 9:25 PM, Christopher Medrela wrote: > 1. Progress: I've made improvements to admin checks. I've also finished > implementing filtering and silencing checks. I've rebased my branch against > master again. > > Excellent - I'll take look and get you some feedback. > 2. I'm afraid that there is too little time to merge django-secure. > September 16 is > suggested 'pencils down' date, so there are only less than two weeks (12 > days) + > one buffer week, because firm 'pencils down' date is September 23. Merging > django-secure in such little time is actually impossible -- we must > through out > this task from schedule. I really apologize for that. > > This is obviously disappointing, but it's better to deliver something complete than a half-attempt. If we can get the core framework into a good state, merging django-secure is a self-contained task that we can address as a follow-up commit. Also -- the GSoC will come to an end, but that doesn't mean your contributions to Django have to… :-) > My plan is that this week I will work at documentation (at this moment > there is > only a draft). I will also try to implement as much "nice to have" things > as > possible. These are: > >- Writing tests which would be nice to have. I mean three tests: > - [#20974], > - test for raising error when ImageField is in use but Pillow is > not installed, > - test for raising error in `BaseCommand.__init__`. >- Moving out checks performed in `__init__`. >- Checking clashes between accessors and GenericForeignKeys. >- Checks for grouped `choices`. > > The second week and the backup week is for deep review. Regarding the > amount > of comments I got during the first review, I guess I need *at least* one > week > for deep review. > This sounds like a reasonable plan -- it's a bunch of relatively easy individual tests, each of which will individually improve the current situation, but each of which could also be omitted if time becomes a factor. When you're ready for another deep review, let me know and I'll run over the code again. [#20974] https://code.djangoproject.com/ticket/20974 > > 3. As I said, I've implemented both filtering and silencing system checks. > > You can run only a subset of checks like that: > > ./manage.py check auth -t tag1 --tag tag2 > > This command will run only these checks which are labeled with `tag1` or > `tag2` > tags and only `auth` app will be validated. If there is no app name, all > apps > are checked. If there is no tag name, all checks are performed. > > Your check function can be labeled in this way: > > from django.core.checks import register, tag > > @tag('mytag') > def my_check_function(apps, **kwargs): > # apps is a list of apps that should be validated; if None, all > apps > # should be checked. > return [] > > register(my_check_function) > > The `tag` decorator works only for entry-point functions, e.g. these one > passed > to `register` function. It doesn't work for checks > functions/methods/classmethods which are called by entry-point functions > (directly or indirectly). > I've just bounced this off Aymeric and he pointed out something interesting. We have a precedent for this in another area -- template tags. You can use a decorator to register a template tag, and that register decorator can also take arguments to control tag naming, etc. Once he pointed this out, it seemed obvious that this is what we should be doing here, too -- after all, the use case is almost identical (registering a function with a central repository, with some optional arguments associated with the registration. So - instead of having a @tag decorator, we should be modifying register to be a decorator, taking optional arguments that lets you specify tags. Depending on how easy the internals for this work, we could do either: @register('mytag') @register('othertag') def my_check_function(apps, **kwargs): … or @register('mytag', 'othertag') def my_check_function(apps, **kwargs): … And, of course, if you don't want to use decorator syntax, you can invoke the decorator directly. To silence a specific error/warning, you need to append its `id` to > SILENCED_SYSTEM_CHECKS setting. The `id` could be any string, but we use > the > following convension: "E001" for core errors, "W002" for core warnings, > "applabel.E001" for errors raised by an custom app (including contrib > apps, i.e. > "sites.E001"). The error/warning number is unique to an app, e.g. > "sites.E001", > "admin.E001" and "E001" are all allowed, but "E001" and "W001" are not OK. > You > should use "E001" and "W002". To create an error/warning with given id, > pass it > as a keyword argument: > > Error('Message', hint=None, obj=invalid_object, id='myapp.E001') > Looks good to me. > 4. More important changes in code: > > - Introduced RAW_SETTINGS_MODULE [1]. I use it in compatibility checks to > te
Re: [GSoC] Revamping validation framework and merging django-secure once again
1. Progress: I've made improvements to admin checks. I've also finished implementing filtering and silencing checks. I've rebased my branch against master again. 2. I'm afraid that there is too little time to merge django-secure. September 16 is suggested 'pencils down' date, so there are only less than two weeks (12 days) + one buffer week, because firm 'pencils down' date is September 23. Merging django-secure in such little time is actually impossible -- we must through out this task from schedule. I really apologize for that. My plan is that this week I will work at documentation (at this moment there is only a draft). I will also try to implement as much "nice to have" things as possible. These are: - Writing tests which would be nice to have. I mean three tests: - [#20974], - test for raising error when ImageField is in use but Pillow is not installed, - test for raising error in `BaseCommand.__init__`. - Moving out checks performed in `__init__`. - Checking clashes between accessors and GenericForeignKeys. - Checks for grouped `choices`. The second week and the backup week is for deep review. Regarding the amount of comments I got during the first review, I guess I need *at least* one week for deep review. [#20974] https://code.djangoproject.com/ticket/20974 3. As I said, I've implemented both filtering and silencing system checks. You can run only a subset of checks like that: ./manage.py check auth -t tag1 --tag tag2 This command will run only these checks which are labeled with `tag1` or `tag2` tags and only `auth` app will be validated. If there is no app name, all apps are checked. If there is no tag name, all checks are performed. Your check function can be labeled in this way: from django.core.checks import register, tag @tag('mytag') def my_check_function(apps, **kwargs): # apps is a list of apps that should be validated; if None, all apps # should be checked. return [] register(my_check_function) The `tag` decorator works only for entry-point functions, e.g. these one passed to `register` function. It doesn't work for checks functions/methods/classmethods which are called by entry-point functions (directly or indirectly). To silence a specific error/warning, you need to append its `id` to SILENCED_SYSTEM_CHECKS setting. The `id` could be any string, but we use the following convension: "E001" for core errors, "W002" for core warnings, "applabel.E001" for errors raised by an custom app (including contrib apps, i.e. "sites.E001"). The error/warning number is unique to an app, e.g. "sites.E001", "admin.E001" and "E001" are all allowed, but "E001" and "W001" are not OK. You should use "E001" and "W002". To create an error/warning with given id, pass it as a keyword argument: Error('Message', hint=None, obj=invalid_object, id='myapp.E001') 4. More important changes in code: - Introduced RAW_SETTINGS_MODULE [1]. I use it in compatibility checks to test if `TEST_RUNNER` setting was overriden [2]. - Introduced `BaseAppCache.get_models_from_apps` method [3]. This method returns all models of given set of apps (or of all apps if None is passed) and is used in `default_checks.py` [4]. - Admin changes are backward compatible. Admin validators are deprecated in favour of admin checks, i.e. `BaseValidator` is deprecated in favour of `BaseModelAdminChecks`. `BaseValidator` is an almost empty class now. `ModelAdmin.validator` class attribute is deprecated in favour of new `checks` attribute. If an ModelAdmin defines `validator`, we are not ignoring it. [1] https://github.com/chrismedrela/django/blob/gsoc2013-checks/django/conf/__init__.py#L139 [2] https://github.com/chrismedrela/django/blob/gsoc2013-checks/django/core/checks/default_checks.py#L33 [3] https://github.com/chrismedrela/django/blob/gsoc2013-checks/django/db/models/loading.py#L296 [4] https://github.com/chrismedrela/django/blob/gsoc2013-checks/django/core/checks/default_checks.py 5. I've left one comment on the pull request. (I mean this one about moving registering admin checks to `autodiscover`.) -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Hi Chris, On Wed, Aug 28, 2013 at 5:16 AM, Christopher Medrela < chris.medr...@gmail.com> wrote: > 1. One of my questions left unanswered on the pull request [1] (I mean > this one > about documentation and `__str__` use.). > I've left a comment on the pull request, I've given the same comment in a previous message on this thread. Short version; I think there's potential for other levels to be used. (that said.. .I get the feeling I think I might have commented on the wrong comment -- it wasn't completely clear from the link you provided) 2. I've finished rewriting admin checks. I've renamed `admin_validation` to > `admin_checks`. I would like you to have a deep look at `fk_name` and > `exclude` checks [2] as well as `inline_formsets` tests [3] (especially > error > messages). > I'm not sure I follow what you've done with the exclude and fk_name checks here -- it looks like you've added some new checks, but I can't make out exactly why those checks are needed. I'm also not wild about the fact you've changed forms code along the way. 2a. "applabel.modellabel[.fieldname]" is the format of error messages for > models > and fields. How should the one for admin look like? I propose > "applabel.ModelAdminClassName". > This sounds reasonable to me; the only other idea I could suggest would be to include a '.admin' namespace in there, so it's clear that the error is part of an admin registration, but I'm not convinced that's needed. > 2b. BaseModelAdmin, ModelAdmin and InlineModelAdmin classes and their > checks > live in separated files. `options.py` defines these classes, but checks > lives > in `checks.py`. We want to have these two issues separated, otherwise class > definitions would become too long. Python does not support open classes, > so we > cannot just add some `_check_*` methods in `checks.py` to existing classes > defined in `options.py`. > > The current approach is that `check` method is defined in `options.py`, > but it > delegates to appropriate functions in `checks.py` [4]. Yes, I use > functions -- > there is no need to have validator class, because we don't need to store > anything in its instances. However, the code is really ugly. I wonder if > there > is any better approach. > Yes, there is. It's called object orientation and classes :-) The existing validation code already does this fairly elegantly - the only difference for your code is that you need to return multiple errors, not just raise an exception. I'm not sure why you've tried to re-invent this particular wheel. Russ %-) -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
1. One of my questions left unanswered on the pull request [1] (I mean this one about documentation and `__str__` use.). 2. I've finished rewriting admin checks. I've renamed `admin_validation` to `admin_checks`. I would like you to have a deep look at `fk_name` and `exclude` checks [2] as well as `inline_formsets` tests [3] (especially error messages). 2a. "applabel.modellabel[.fieldname]" is the format of error messages for models and fields. How should the one for admin look like? I propose "applabel.ModelAdminClassName". 2b. BaseModelAdmin, ModelAdmin and InlineModelAdmin classes and their checks live in separated files. `options.py` defines these classes, but checks lives in `checks.py`. We want to have these two issues separated, otherwise class definitions would become too long. Python does not support open classes, so we cannot just add some `_check_*` methods in `checks.py` to existing classes defined in `options.py`. The current approach is that `check` method is defined in `options.py`, but it delegates to appropriate functions in `checks.py` [4]. Yes, I use functions -- there is no need to have validator class, because we don't need to store anything in its instances. However, the code is really ugly. I wonder if there is any better approach. 3. I've created a new ticket [#20974] about lack of mysql-specific checks. 4. I've rebased my branch against master. On Fri, Aug 16, 2013 at 7:45 AM, Russell Keith-Magee > wrote: > >> 8. I've added a new check. If you're using a `GenericRelation` but there >>> is no >>> `GenericForeignKey` on the target model, an warning is raised. This >>> check was >>> implemented in this commit [9]. It uses `vars` builtin function to check >>> if the >>> target model has a `GenericForeignKey`. This is ugly, but I don't see a >>> better >>> approach. >> >> [9] >>> https://github.com/chrismedrela/django/commit/ab65ff2b6d6346407a11a72c072e358c7b518cf9#L1R397 >>> >> Hrm. I don't really like this, but I'm not sure I have a better option. A >> better approach would be to have GFKs turn up in get_fields, but it isn't >> your responsibility to fix the internal problems of Generic Foreign Keys. >> If we have to live with this, then we should at the very least document it >> as a FIXME, pointing at the underlying problem with _meta handling of GFKs. > > > > Michal Petruca just mailed the django-dev list with some discussion about > changes he wants to make in his own GSoC project, which drew my attention > to something I'd forgotten about. > Does _meta.virtual_fields -- contain the information you need? It looks > like it should. 5. Yes, you're right, `virtual_fields` is what I need. 6. Now I'm implementing filtering checks. [1] https://github.com/django/django/pull/1364 [2] https://github.com/chrismedrela/django/blob/gsoc2013-checks/django/contrib/admin/checks.py#L673 [3] https://github.com/chrismedrela/django/blob/gsoc2013-checks/tests/inline_formsets/tests.py#L113 [4] https://github.com/chrismedrela/django/blob/gsoc2013-checks/django/contrib/admin/checks.py [#20976] https://code.djangoproject.com/ticket/20976 -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Progress: - Converted `BaseCommand.verbosity` from bytestring into an unicode. - Integrated compatibility checks. - Deprecated "validate" command. This command delegates to "check" command now. Changed "check" command -- it performs all system checks, including model validation and compatibility checks. - Added test for check of uniqueness of field combination under ForeignObject. Now I'm working at rewriting admin checks (at gsoc2013-checks-admin branch [1]). [1] https://github.com/chrismedrela/django/tree/gsoc2013-checks-admin -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On Fri, Aug 16, 2013 at 7:45 AM, Russell Keith-Magee < russ...@keith-magee.com> wrote: > > 8. I've added a new check. If you're using a `GenericRelation` but there >> is no >> `GenericForeignKey` on the target model, an warning is raised. This check >> was >> implemented in this commit [9]. It uses `vars` builtin function to check >> if the >> target model has a `GenericForeignKey`. This is ugly, but I don't see a >> better >> approach. >> >> [9] >> https://github.com/chrismedrela/django/commit/ab65ff2b6d6346407a11a72c072e358c7b518cf9#L1R397 >> > > Hrm. I don't really like this, but I'm not sure I have a better option. A > better approach would be to have GFKs turn up in get_fields, but it isn't > your responsibility to fix the internal problems of Generic Foreign Keys. > If we have to live with this, then we should at the very least document it > as a FIXME, pointing at the underlying problem with _meta handling of GFKs. > > Michal Petruca just mailed the django-dev list with some discussion about changes he wants to make in his own GSoC project, which drew my attention to something I'd forgotten about. Does _meta.virtual_fields -- contain the information you need? It looks like it should. Russ %-) -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Hi Christopher, On Wed, Aug 14, 2013 at 11:26 PM, Christopher Medrela < chris.medr...@gmail.com> wrote: > *Progress.* > > - Deprecated `requires_model_validation` flag and `validate` method (both > `BaseCommand` members) in favour of new `requires_system_checks` flag and > `check` method. > > - Finished working at `contenttypes` tests. > > - Improved code thanks to Preston Holmes comments. Deleted dead code and > added > some new tests to improve code coverage. > > It'd be nice to have checks of clashes between GenericForeignKey and > accessors. I didn't implemented it because little time left and I need to > hurry up. > This one is entirely understandable; it's a complex area, but it's an area that isn't currently being validated. If someone wants to contribute validation for this once this GSoC project is over, it would make a nice contribution. It would be helpful if you log this sort of known omission as a bug, and flag the bug as an "easy pickings" -- that means when we get to a sprint and a developer who is new to Django contribution is looking for something to do, we can point them at this problem and suggest they take a look. > When it was easy, I've added new tests to improve code coverage. However, > there > are still some tests that would be nice to have: > > - Test for raising an error while using an unique varchars longer than 255 > characters under MySQL backend. [1] > > - Test for `ImageField`. When `Pillow` is not installed and `ImageField` > is in > use, an error should be raised. This should be tested. [2] > > - Test for raising warning/ImproperlyConfigured in `BaseCommand.__init__`. > [3] > > [1] > https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/db/backends/mysql/validation.py#L6 > [2] > https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/db/models/fields/files.py#L447 > [3] > https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/core/management/base.py#L202 > > Same goes for these three -- if you can raise a ticket for these testing omissions, we can get someone else to look at them at a later date. > *Filtering and silencing errors/warnings.* We need two features: ability > to run only a subset of checks (aka filtering errors) and ability to > silence > some errors. > > Silencing is easy. Every warning will have a unique name like "W027". The > format > of the name is letter "W" followed by a unique number. The system check > framework is open ended and third-party apps can register its own checks. > For > warnings raised by these apps, "Wnumber.applabel" (e.g. "W001.myapp") > style will > be used. Of course, everything can be changed and I'm open to yours > suggestions, > so feel free to comment and criticize it. > > There will be a new setting called `SILENCED_WARNINGS`. If you want to > silence a > warning, you put its name in this setting, e.g.: > > SILENCED_ERRORS = [ > 'W027', > ] > I'd suggest SILENCED_SYSTEM_CHECKS here -- in your example here you've already used SILENCED_WARNINGS and SILENCED_ERRORS, which points out the potential for confusion. Including the full name is also helpful, just to make clear that we're not silencing HTTP500s (other any other errors) here. As for the numbering -- I'd be inclined to use "myapp.W123", not the other way around -- put the namespace in front of the error code. Only light messages (warnings, infos and debugs) can be silenced. > > Running only a subset of check is a more complex task. We can associate > every > check with a set of tags (that can be done while registering checks). To > run > only checks tagged "mytag" or "mysecondtag", you need to type: > > manage.py check mytag mysecondtag > > However, we would like to run checks of an app (or a set of apps): > > manage.py check auth admin > > This is hard, because checks are no longer app-specific. I propose to > solve this > by passing an `apps` optional keyword argument to every check function. The > function should only validate specified apps (or all apps if apps==None). > The > only problem is how to determine if we deal with a tag or app name? > Consider > this command: > > manage.py check auth mytag > > This should run all checks tagged "mytag". Only messages for `auth` app > should > be reported. I propose to collect all tags while registering check > functions and > if a string is one of them, then interpret it as a tag, otherwise assume > it's an > app name (and check if an app with given name exists). > I'd be inclined to use command line arguments for the tags here -- you don't want to have ambiguity between the "mytag" tag and the mytag app. ./manage.py check mytag --check=mytag --check=upgrade --check=security (not completely sure I like the --check name itself, but I hope the point is clear) *Problems/questions.* > > 1. We decided to mimic message and logging framework, so every error is > tagged > wit
Re: [GSoC] Revamping validation framework and merging django-secure once again
*Progress.* - Deprecated `requires_model_validation` flag and `validate` method (both `BaseCommand` members) in favour of new `requires_system_checks` flag and `check` method. - Finished working at `contenttypes` tests. - Improved code thanks to Preston Holmes comments. Deleted dead code and added some new tests to improve code coverage. It'd be nice to have checks of clashes between GenericForeignKey and accessors. I didn't implemented it because little time left and I need to hurry up. When it was easy, I've added new tests to improve code coverage. However, there are still some tests that would be nice to have: - Test for raising an error while using an unique varchars longer than 255 characters under MySQL backend. [1] - Test for `ImageField`. When `Pillow` is not installed and `ImageField` is in use, an error should be raised. This should be tested. [2] - Test for raising warning/ImproperlyConfigured in `BaseCommand.__init__`. [3] [1] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/db/backends/mysql/validation.py#L6 [2] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/db/models/fields/files.py#L447 [3] https://github.com/chrismedrela/django/blob/149800a8136ce839903f0fe9af7f548973da9244/django/core/management/base.py#L202 *Filtering and silencing errors/warnings.* We need two features: ability to run only a subset of checks (aka filtering errors) and ability to silence some errors. Silencing is easy. Every warning will have a unique name like "W027". The format of the name is letter "W" followed by a unique number. The system check framework is open ended and third-party apps can register its own checks. For warnings raised by these apps, "Wnumber.applabel" (e.g. "W001.myapp") style will be used. Of course, everything can be changed and I'm open to yours suggestions, so feel free to comment and criticize it. There will be a new setting called `SILENCED_WARNINGS`. If you want to silence a warning, you put its name in this setting, e.g.: SILENCED_ERRORS = [ 'W027', ] Only light messages (warnings, infos and debugs) can be silenced. Running only a subset of check is a more complex task. We can associate every check with a set of tags (that can be done while registering checks). To run only checks tagged "mytag" or "mysecondtag", you need to type: manage.py check mytag mysecondtag However, we would like to run checks of an app (or a set of apps): manage.py check auth admin This is hard, because checks are no longer app-specific. I propose to solve this by passing an `apps` optional keyword argument to every check function. The function should only validate specified apps (or all apps if apps==None). The only problem is how to determine if we deal with a tag or app name? Consider this command: manage.py check auth mytag This should run all checks tagged "mytag". Only messages for `auth` app should be reported. I propose to collect all tags while registering check functions and if a string is one of them, then interpret it as a tag, otherwise assume it's an app name (and check if an app with given name exists). *Problems/questions.* 1. We decided to mimic message and logging framework, so every error is tagged with a level. Its value is an integer indicating how important and serious is a message. There are five predefined values: CRITICAL, ERROR, WARNING, INFO, DEBUG. However, Preston Holmes noticed that this is unpractical because we need only errors and warnings. I think we should discuss again if it's worth mimicing these frameworks. 2. On the pull request we started discussing names. "System check" is better than "check" but it suggest that it's connected with hardware. Preston proposed "Startup checks". I don't have strong opinion. To be honest, I don't thing "startup checks" is much more better than "system checks" so I will leave it as it is, but I'm still open to suggestions and I would like to see yours opinion. 3. There are some problems with moving custom User model checks. Their first source was that some tests override `INSTALLED_APPS` setting but don't override list of registered checks. So checks of `auth` app were registered (because the app was imported by some other tests), but this app wasn't installed. This ended in an error, because checks of `auth` try to load User model which is, by default, `auth.User` and `auth` is not installed. I've solved this by overriding list of registered checks (I've introduced `override_system_checks` decorator). However, there is still one red test -- `admin_scripts.test_complex_app` [4]. The problem is that this test spawns a new Django process and the decorator cannot affect this new process. This test installs two apps (`complex_app` and `simple_app`) and doesn't install `auth` app. However, `auth` app is imported, because `complex_app` imports `admin` app which imports `aut
Re: [GSoC] Revamping validation framework and merging django-secure once again
On Wed, Aug 7, 2013 at 4:03 AM, Christopher Medrela wrote: > I'm still working at polishing after reviewing. I've deprecated > `requires_model_validation` and `validate`. I've started at adding tests > for > contenttype fields: `GenericForeignKey` and `GenericRelation`. > > I've updated gsoc2013-checks-review branch [1]. Now it's the same as > gsoc2013-checks branch [2]. I will push new work to the latter while the > former > will remain unchanged. I'm working at contenttype tests in > gsoc2013-checks-contenttypes [3] branch. The work is not finished yet and > there > are some failing tests. > > @Shai Berger: Thank you for creating the ticket. I'm sorry that I > procrastinated accepting it -- I finally did it and proposed a patch. [4] > > [1] https://github.com/chrismedrela/django/tree/gsoc2013-checks-review > [2] https://github.com/chrismedrela/django/tree/gsoc2013-checks > [3] > https://github.com/chrismedrela/django/tree/gsoc2013-checks-contenttypes > [4] https://code.djangoproject.com/ticket/20814 > > Questions: > > 1. Output formatting. We decided that every error/warning will take one > line plus > additional one for a hint if it's provided. The justification is that a > Django > user may type "grep HINT" to filter all hints. But now I think it's > unpractical > since the lines with hints doesn't say which object is invalid. So we can: > (1) > put hint in the same line as the error message or (2) change the format to > sth > like this: > > applabel.modellabel: Error message. > applabel.modellabel: HINT: Hint. > For my money, the label on the second line isn't needed. I know I gave grep as a use case, but in retrospect, that's probably isn't as useful as you'd think. The reason to have the newline break with an indent is so that theres a visual cue for any hint. It makes the hint stand out. Putting a prefix on that line obscures this. applabel.modellabel: Error message HINT: the hint > 2. Is it allowed to use `GenericRelation` pointing to a model if the model > lacks > `GenericForeignKey`? > In theory, I suppose you could, as long as there was a pair of object and content type fields. However, in practice, I don't think this is a very likely use case. I'd be completely comfortable if you confirmed the existence of a GenericForeignKey. Raising this as a warning might be a good approach here. 3. I've added unicode_literals import to django/core/management.py but this > affected `BaseCommand.verbosity` default value. In order not to break > commands, > I left the attribute as a bytestring [5]. Changing it to an unicode breaks > some admin_scipts tests, i. e. `CommandTypes.test_app_command` expects in > stdout: > > ... options=[('pythonpath', None), ... ('verbosity', '1')] > > while the command prints: > > ... options=[('pythonpath', None), ... ('verbosity', u'1')] > I can see what you're doing here, but I'm not sure it's needed. u'1' == '1' under Python 2.7, and under Python 3 it's all unicode anyway. 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
I'm still working at polishing after reviewing. I've deprecated `requires_model_validation` and `validate`. I've started at adding tests for contenttype fields: `GenericForeignKey` and `GenericRelation`. I've updated gsoc2013-checks-review branch [1]. Now it's the same as gsoc2013-checks branch [2]. I will push new work to the latter while the former will remain unchanged. I'm working at contenttype tests in gsoc2013-checks-contenttypes [3] branch. The work is not finished yet and there are some failing tests. @Shai Berger: Thank you for creating the ticket. I'm sorry that I procrastinated accepting it -- I finally did it and proposed a patch. [4] [1] https://github.com/chrismedrela/django/tree/gsoc2013-checks-review [2] https://github.com/chrismedrela/django/tree/gsoc2013-checks [3] https://github.com/chrismedrela/django/tree/gsoc2013-checks-contenttypes [4] https://code.djangoproject.com/ticket/20814 Questions: 1. Output formatting. We decided that every error/warning will take one line plus additional one for a hint if it's provided. The justification is that a Django user may type "grep HINT" to filter all hints. But now I think it's unpractical since the lines with hints doesn't say which object is invalid. So we can: (1) put hint in the same line as the error message or (2) change the format to sth like this: applabel.modellabel: Error message. applabel.modellabel: HINT: Hint. 2. Is it allowed to use `GenericRelation` pointing to a model if the model lacks `GenericForeignKey`? 3. I've added unicode_literals import to django/core/management.py but this affected `BaseCommand.verbosity` default value. In order not to break commands, I left the attribute as a bytestring [5]. Changing it to an unicode breaks some admin_scipts tests, i. e. `CommandTypes.test_app_command` expects in stdout: ... options=[('pythonpath', None), ... ('verbosity', '1')] while the command prints: ... options=[('pythonpath', None), ... ('verbosity', u'1')] [5] https://github.com/chrismedrela/django/blob/09579fdafc05dea90db41f519b26655010a50ac2/django/core/management/base.py#L169 -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Unfortunately, I'm a bit late. I didn't suspected that polishing code after review takes so much time. Lots of my work from last Wednesday was small improvements, but there are some vital changes: The API will be consistent with API of logging module or message framework. Being consistent means that system check framework will be familiar to lots of developers. The change is that every message has its `level` now -- one of CRITICAL, ERROR, WARNING, INFO or DEBUG integer values. Django users may add their own levels (like SUPER_CRITICAL) since `level` is an integer. This change affected output format. All messages are sorted by level (and then by source of problem), so critical errors are first on the list and there is no opportunity to miss an important error in the middle of long list of warnings. There is some stuff that still need to be done as part of reviewing, i. e. deprecating `BaseCommand.validate` in favour of `check` or adding tests for `GenericForeignKey`. After finishing polishing I will focus on moving custom User model checks to auth apps, and then on rewritting admin checks. Note that there is a lot of discussion on the pull request [1]. [1] https://github.com/django/django/pull/1364 -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On Thursday 25 July 2013 08:37:06 Russell Keith-Magee wrote: > > Could I get you to open this as a ticket so that it isn't forgotten? > https://code.djangoproject.com/ticket/20814 Thanks, Shai. -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Hi Shai, Could I get you to open this as a ticket so that it isn't forgotten? Russ %-) On Thu, Jul 25, 2013 at 8:12 AM, Shai Berger wrote: > Hi Christopher, > > While you're dealing with model validation, I wonder if you can take a > look at > this little example -- a minor failure in the current model validation: > > class General(models.Model): > name = models.CharField(max_length=30) > > class Special(General): > pass > > class SpecialDetail(models.Model): > parent = models.ForeignKey(Special) > target = models.ForeignKey(General) > > > Model validation fails (as it should), but the error message is > > specialdetail: accessor for field 'parent' clashes with > 'Special.specialdetail_set' > > when in fact the clash is with 'General.specialdetail_set' generated by the > 'target' field (and inherited by Special). Of course, when isolated like > this, > it is very easy to spot where the real problem is, but when the models > have a > little more content in them, this can be (and actually was) quite > perplexing. > > Thanks, > Shai. > > -- > 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. > For more options, visit https://groups.google.com/groups/opt_out. > > > -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Hi Christopher, While you're dealing with model validation, I wonder if you can take a look at this little example -- a minor failure in the current model validation: class General(models.Model): name = models.CharField(max_length=30) class Special(General): pass class SpecialDetail(models.Model): parent = models.ForeignKey(Special) target = models.ForeignKey(General) Model validation fails (as it should), but the error message is specialdetail: accessor for field 'parent' clashes with 'Special.specialdetail_set' when in fact the clash is with 'General.specialdetail_set' generated by the 'target' field (and inherited by Special). Of course, when isolated like this, it is very easy to spot where the real problem is, but when the models have a little more content in them, this can be (and actually was) quite perplexing. Thanks, Shai. -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On Wed, Jul 24, 2013 at 4:17 AM, Christopher Medrela < chris.medr...@gmail.com> wrote: > Progress: I've implemented registering entry points. Now there is > `django.core.checks.register(callable)` function. There is no > `run_in_development` and `run_in_production` arguments. I've also rewritten > mechanism of triggering checking framework -- `BaseCommand.validate` calls > `django.core.checks.run_checks` instead of `get_validation_errors` (which > was > deleted) now. > > Questions: > > 1. BaseCommand.validate have an `app` optional argument to run validation > of a > particular app (if app==None then all checks are performed). > Unfortunately, with > the current checking framework, we are not able to run checks for a > particular > app. The attribute isn't used anywhere in Django except for three tests > [1] and > I don't think that this is heavily used by third party commands. So I > propose to > deprecate this attribute if it's possible. > > [1] > https://github.com/django/django/blob/master/django/contrib/auth/tests/test_management.py#L176 > What's the technical cause of needing to drop this feature? Is it because checks are no longer guaranteed to be "app specific"? Or is there some other cause? At the end of the day, I don't especially mind if we have to drop this as a feature for the new checks framework. Checks really are something that should be run in their entirety, and if you get any errors, then . However, I can see how some people might have use for running a subset of checks against a subset of the project. If we need to drop this feature, I want to make sure we're not dropping it for a good reason, not just because it would be harder to implement. > 2a. Warnings are printed with bold, yellow foreground now (errors use red > color). Is it a good choice? > Sounds good to me. > 2b. The rules of formatting error messages are that the error message > (that may > be multiline) is followed by the hint in the next line, i. e.: > > app_label.model_label: Error message. > Error message can be multilined. In that case, the first line should > be a short description and the rest should be a long description. > HINT: Hints are printed in a new line. > > If hint is missing then there is no last line. If the invalid object is a > field > or a manager, then the error message starts with > `app_label.model_label.field_or_manager_name: `. If it's neither a model, a > manager nor a field, then '?: ' is printed. Do you have any opinion about > this > style? > Sounds good to me. My only additional comment would be to indent the 2nd+ lines slightly, so that you can easily identify the first lines of a large number of multi-line errors/warnings. It might also be worth having different indentation levels for continuation, and for starting the hint. For example: app_label.model_label: Error message. Error message can be multilined. In that case, the first line should be a short description and the rest should be a long description. HINT: Hints are printed in a new line. If hints can be multiline, then they should also be indented to a new level 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Progress: I've implemented registering entry points. Now there is `django.core.checks.register(callable)` function. There is no `run_in_development` and `run_in_production` arguments. I've also rewritten mechanism of triggering checking framework -- `BaseCommand.validate` calls `django.core.checks.run_checks` instead of `get_validation_errors` (which was deleted) now. Questions: 1. BaseCommand.validate have an `app` optional argument to run validation of a particular app (if app==None then all checks are performed). Unfortunately, with the current checking framework, we are not able to run checks for a particular app. The attribute isn't used anywhere in Django except for three tests [1] and I don't think that this is heavily used by third party commands. So I propose to deprecate this attribute if it's possible. [1] https://github.com/django/django/blob/master/django/contrib/auth/tests/test_management.py#L176 2a. Warnings are printed with bold, yellow foreground now (errors use red color). Is it a good choice? 2b. The rules of formatting error messages are that the error message (that may be multiline) is followed by the hint in the next line, i. e.: app_label.model_label: Error message. Error message can be multilined. In that case, the first line should be a short description and the rest should be a long description. HINT: Hints are printed in a new line. If hint is missing then there is no last line. If the invalid object is a field or a manager, then the error message starts with `app_label.model_label.field_or_manager_name: `. If it's neither a model, a manager nor a field, then '?: ' is printed. Do you have any opinion about this style? Now I will focus on moving custom user model checks from Model class to auth app (that should be easy), then rewriting admin checks and finally finishing first iteration (mainly polishing docs). -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
I've created a pull request [1] to make deep review easier. I've rolled out tests we were talking about and reverted `validate_field` -- there exist both `check_field` and `validate_field`, the first calling the second. [1] https://github.com/django/django/pull/1364 -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
On Mon, Jul 15, 2013 at 8:17 PM, Christopher Medrela < chris.medr...@gmail.com> wrote: > Progress: I've implemented manager checks. > > The schedule for the next 4 weeks is: > > 1. Manager checks (ref 4.1.7)(done)(0.5 week). > 2. Entry point registering (ref 4.1.5) & rewriting mechanism of triggering >checking framework (ref 4.1.9)(1.5 week). > 3. Moving custom User model checks (ref 4.1.6)(0.5 week). > 4. Rewriting AdminModel checks and tests (ref 4.1.8)(1 week). > 5. Polishing doc (ref 4.1.10)(0.5 week). > > (ref 4.1.x are references to my original proposal [1]) > > [1] https://gist.github.com/chrismedrela/82cbda8d2a78a280a129) > > Let's talk about public API to register your own validation stuff. There > will > be `register(callable)` function in `django.core.checks`. It will return > nothing. The callables will be stored in a global variable. They should > obey > the same contract as `check` methods: they allow for `**kwargs` and return > list of errors and warnings. All registered callables will be called during > checking phase (i. e. when you type `manage.py validate` or before running > server). > I think you're on the right track here. The only other viable approach I can think of would be a setting (SYSTEM_CHECKS) that is a list of functions to be invoked. However, this means installing a new app that contained a set of checks would mean modifying multiple settings (INSTALLED_APPS, SYSTEM_CHECKS, and possibly others for templates etc) This is also true of the registration approach -- the only difference is where you put the call to register. However, looking into the medium-term future, app refactor will eventually land, and at that point, we'll have a natural point to put all these registrations -- the app definition itself would be able to register the additional checks. To that end, I think a registration based approach will ultimately be better. And it isn't something that most users will be exposed to anyway -- all the pre-existing checks can be pre-registered, and admin can do it's registration as part of the autodiscover process. This API allows us to register, among other things, app-specific checks. But > it's not necessary to write an app in order to do some checks. > > `register` function will have two optional parameters: `run_in_production` > (default: False) and `run_in_development` (default: True) that let you > specify > when the callable should be called. Most checks should be performed only in > development environment (which is equivalent to DEBUG==True). Security > checks > makes sense only in production environment (<==> DEBUG==False). > > I would be happy if I could hear your opinion! > I don't think we need to worry about 'in production' level checks at time of registration. The default behaviour should always be to run all checks when ./manage.py check is invoked. There are some very specific checks (such as security checks) that only make sense in production, but they're the exception, not the rule. These checks could be skipped using a decorator when they're defined - essentially: def only_in_production(fn): def _dec(*args, **kwargs): if settings.DEBUG == False: return fn(*args, **kwargs) return [] return _dec @only_in_production def security_checks(…): … This decorator could be applied to the security check, so it becomes a no-op when it is executed in development. This can be determined by the check itself - I can't think of any circumstance where a user registering a check would need to have control over whether a check would be executed at all. 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Hi Chris, On Fri, Jul 12, 2013 at 7:45 PM, Christopher Medrela < chris.medr...@gmail.com> wrote: > Finally, I've rewritten all model and field tests and checks. Some new > tests > were written. First draft of checking framework documentation was written. > All > tests passes (at least at my computer). I've rebased my branch > (`gsoc2013-checks` [1]) against master. The branch is ready to deep review. > > [1] https://github.com/chrismedrela/django/tree/gsoc2013-checks > This is looking good! If you issue a pull request for this branch, I'll add some detailed comments there. When you make the pull request, make sure you add a comment noting that this is a work in progress associated with the GSoC, and isn't a candidate for merging (yet!). > I've created pull request [2] for ticket [#19934]. The patch clarifies that > importing `django.utils.image` cannot result in ImportError. I've also > created > ticket [#20735] about confusing ManyToManyField documentation. There is a > pull > request too [3]. Both pull requests were merged. > > [2] https://github.com/django/django/pull/1349 > [3] https://github.com/django/django/pull/1350 > [#19934] https://code.djangoproject.com/ticket/19934 > [#20735] https://code.djangoproject.com/ticket/20735 > 1. I wrote some tests [4] where nose tests generators [5] would be really > helpful. I tried to emulate them. Is the code I wrote a good approach? > > [4] > https://github.com/chrismedrela/django/blob/1c14c674aa52c666ec8742c72657d5dad2efe0cb/tests/invalid_models/tests.py#L690 > [5] > https://nose.readthedocs.org/en/latest/writing_tests.html#test-generators > I can see what you're doing here, but it's probably not the best approach. As implemented, there are two problems: 1) there's going to be a redundant call to setUp and tearDown 2) you only get one opportunity for a failure in all the accessor_check tests. Given that you're only dealing with 6 tests (2 targets x 3 relatives) here, I'd be inclined to manually roll them out. When we've got Python 3.4 as a minimum dependency, we can start using subunit tests [1], but until then, we can live with a small amount of manual code rollout. [1] http://docs.python.org/dev/library/unittest.html#distinguishing-test-iterations-using-subtests > 2. Two m2m through the same model are forbidden. The new error message is: > > applabel.modelname: Two m2m relations through the same model. > The model has two many-to-many relations through the intermediary > Membership > model, which is not permitted. > (No hint.) > > > Is there any reasonable hint we can provide except for suggesting > duplicating > the intermediary model? > The hint is optional, so there's not much point having a hint unless it's actually providing useful information that isn't implicit in the error message. You can't have 2 m2m relations through the same intermediate model. The fix - don't have 2 m2m relations through the same intermediate model. The only practical advice we could give here would be to duplicate the intermediate model, but that doesn't strike me as something that is likely to be correct advice under most circumstances, so I'm hesitant to provide it as default advice. > 3. I've deleted `app_errors` [6] because it wasn't used anywhere (inside > Django). Was it left by accident or was there a reason for that? > > [6] > https://github.com/chrismedrela/django/commit/3b221caefc151f7750a322e18798c22343daba6f > I'm not aware of anywhere that it is being used; I'm guessing it exists for historical reasons, but I have no idea what those are. If the tests still pass when you've removed this code, I see no reason to keep it around. > 4. I've renamed DatabaseValidation.validate_field to check_field and > changed its > arguments. They were: `errors` -- ModelErrorCollection [7], `opts` -- meta > of > the model of the field and `f` -- the field; The method returned None. Now > there > is only `field` argument and the method returns list of errors. It's not > public > API, but we should go through deprecation process. > > How can we do that regarding changed arguments? We can call old > `validate_field` > method where the backend specific validation is triggered [8]. By default, > the > old method will call `check_field` method. The old one need to transform > list of > errors into strings and add them to `errors` list. The problem is that at > [8] we > need to convert the strings again to errors. That cannot be done in a > graceful > way. Is there any other way to achieve deprecation? > > I'd suggest that we ship both APIs -- validate_field() unmodified to support older codebases (but internally documented to indicate that we're moving away from that approach), and a new check_field() method that does things the New Way. As a migration aid, the default implementation of check_field() could invoke validate_field() and just convert the output into the new format -- it should be possible to retrieve all the required arguments from the data you already have
Re: [GSoC] Revamping validation framework and merging django-secure once again
Progress: I've implemented manager checks. The schedule for the next 4 weeks is: 1. Manager checks (ref 4.1.7)(done)(0.5 week). 2. Entry point registering (ref 4.1.5) & rewriting mechanism of triggering checking framework (ref 4.1.9)(1.5 week). 3. Moving custom User model checks (ref 4.1.6)(0.5 week). 4. Rewriting AdminModel checks and tests (ref 4.1.8)(1 week). 5. Polishing doc (ref 4.1.10)(0.5 week). (ref 4.1.x are references to my original proposal [1]) [1] https://gist.github.com/chrismedrela/82cbda8d2a78a280a129) Let's talk about public API to register your own validation stuff. There will be `register(callable)` function in `django.core.checks`. It will return nothing. The callables will be stored in a global variable. They should obey the same contract as `check` methods: they allow for `**kwargs` and return list of errors and warnings. All registered callables will be called during checking phase (i. e. when you type `manage.py validate` or before running server). This API allows us to register, among other things, app-specific checks. But it's not necessary to write an app in order to do some checks. `register` function will have two optional parameters: `run_in_production` (default: False) and `run_in_development` (default: True) that let you specify when the callable should be called. Most checks should be performed only in development environment (which is equivalent to DEBUG==True). Security checks makes sense only in production environment (<==> DEBUG==False). I would be happy if I could hear your opinion! -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Finally, I've rewritten all model and field tests and checks. Some new tests were written. First draft of checking framework documentation was written. All tests passes (at least at my computer). I've rebased my branch (`gsoc2013-checks` [1]) against master. The branch is ready to deep review. [1] https://github.com/chrismedrela/django/tree/gsoc2013-checks I've created pull request [2] for ticket [#19934]. The patch clarifies that importing `django.utils.image` cannot result in ImportError. I've also created ticket [#20735] about confusing ManyToManyField documentation. There is a pull request too [3]. Both pull requests were merged. [2] https://github.com/django/django/pull/1349 [3] https://github.com/django/django/pull/1350 [#19934] https://code.djangoproject.com/ticket/19934 [#20735] https://code.djangoproject.com/ticket/20735 1. I wrote some tests [4] where nose tests generators [5] would be really helpful. I tried to emulate them. Is the code I wrote a good approach? [4] https://github.com/chrismedrela/django/blob/1c14c674aa52c666ec8742c72657d5dad2efe0cb/tests/invalid_models/tests.py#L690 [5] https://nose.readthedocs.org/en/latest/writing_tests.html#test-generators 2. Two m2m through the same model are forbidden. The new error message is: applabel.modelname: Two m2m relations through the same model. The model has two many-to-many relations through the intermediary Membership model, which is not permitted. (No hint.) Is there any reasonable hint we can provide except for suggesting duplicating the intermediary model? 3. I've deleted `app_errors` [6] because it wasn't used anywhere (inside Django). Was it left by accident or was there a reason for that? [6] https://github.com/chrismedrela/django/commit/3b221caefc151f7750a322e18798c22343daba6f 4. I've renamed DatabaseValidation.validate_field to check_field and changed its arguments. They were: `errors` -- ModelErrorCollection [7], `opts` -- meta of the model of the field and `f` -- the field; The method returned None. Now there is only `field` argument and the method returns list of errors. It's not public API, but we should go through deprecation process. How can we do that regarding changed arguments? We can call old `validate_field` method where the backend specific validation is triggered [8]. By default, the old method will call `check_field` method. The old one need to transform list of errors into strings and add them to `errors` list. The problem is that at [8] we need to convert the strings again to errors. That cannot be done in a graceful way. Is there any other way to achieve deprecation? [7] https://github.com/django/django/blob/master/django/core/management/validation.py#L11 [8] https://github.com/chrismedrela/django/blob/1c14c674aa52c666ec8742c72657d5dad2efe0cb/django/db/models/fields/__init__.py#L710 -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
I'm finishing making field tests green. I'm a bit late, because the schedule says that I should finish this job by the end of the previous week. My excuse is that I had exams in the previous week so I couldn't focus on GSoC. Fortunately, I passed all exams and now I can work full time on GSoC and catch up. This week I will finish field tests (there are still 5 red tests and documentation needs to written) and I will start working on model tests. Today I had a conversation with Russell and we decided to simplify some issues. For example, `check_*` methods won't yield errors -- they will just return list of errors. What's more, `check` method will call all `check_*` methods explicitly (now it uses introspection to find all methods starting with `check_`). -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
Hello! I've deleted old `gsoc2013-verification` branch. Follow the new `gsoc2013-checks` branch [1]. [1] https://github.com/chrismedrela/django/tree/gsoc2013-checks What did I do? I've rewritten field tests living in `django.tests.invalid_models` package [2], inside `tests.py` file [3]. I've renamed `invalid_models.invalid_models` submodule into `old_invalid_models`. I've created new `invalid_models` submodule; it contains models required by the rewritten tests. [2] https://github.com/chrismedrela/django/tree/gsoc2013-checks/tests/invalid_models [3] https://github.com/chrismedrela/django/blob/gsoc2013-checks/tests/invalid_models/tests.py#L65 Now I'm working on rewritting field checks. Now all checks are done in `get_validation_errors` function [4]. They will be moved to relevant `Field` subclasses during the current week. I've introduced `Warning` and `Error` classes inside `django.core.checks` package. [4] https://github.com/chrismedrela/django/blob/gsoc2013-checks/django/core/management/validation.py#L22 Now I'm taking exams at university. I will have last exam on Friday (I hope), so next week it will be easier to focus on GSoC and I will work full time. Good-bye! -- 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. For more options, visit https://groups.google.com/groups/opt_out.
Re: [GSoC] Revamping validation framework and merging django-secure once again
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
Re: [GSoC] Revamping validation framework and merging django-secure once again
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. **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. **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.
Re: [GSoC] Revamping validation framework and merging django-secure once again
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 -- 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.