Check Framework Feedback

146 views
Skip to first unread message

Thomas Güttler

unread,
Feb 5, 2014, 9:13:25 AM2/5/14
to Django Developers
Hi,

here is some feedback to the check framework, documented at:

https://docs.djangoproject.com/en/dev/ref/checks/

---


{{{
Error('error message', None) # Good
}}}

I don't think this is "good". The "hint" should be optional. The above line
is ugly, since you don't know why there is second argument (None).

---

https://docs.djangoproject.com/en/dev/ref/django-admin/#django-admin-check

{{{
...

--tag <tagname>

The system check framework performs many different types of checks.
}}}

Which tags are available from django?

How can I list the available tags (django tags and my tags)?

How can I list the available checks?

If a check needs two minutes, it is not wise to run it on "runserver"
on development machines. Is there a way to define which tests should
be included or excluded depending on the context. For example: if
"runserver" gets called don't call tests with have tag "foo".

----

https://docs.djangoproject.com/en/dev/ref/checks/#registering-and-labeling-checks

{{{
@register('compatibility')
}}}

I don't like strings like this, since it can contain typos.

Are there objects for common tags? Example:

{{{
@register(Tag.COMPATIBILITY)
}}}



----

Thank you for this check framework. I like it.


Regards,
Thomas Güttler

--
Thomas Guettler, http://www.tbz-pariv.de/

Russell Keith-Magee

unread,
Feb 5, 2014, 6:14:02 PM2/5/14
to Django Developers

Hi Thomas,

On Wed, Feb 5, 2014 at 10:13 PM, Thomas Güttler <h...@tbz-pariv.de> wrote:
Hi,

here is some feedback to the check framework, documented at:

https://docs.djangoproject.com/en/dev/ref/checks/

---


{{{
Error('error message', None) # Good
}}}

I don't think this is "good". The "hint" should be optional. The above line
is ugly, since you don't know why there is second argument (None).

This was intended as a case of enforcing explicit > implicit. The next line in the documentation reads: 

{{{
Error('error message', hint=None)  # Better
}}}

suggesting that the preferred syntax is to explicitly state that there isn't a hint.

However, I can see how this might be slightly contrary to expectation for Python developers generally. I'd be interested in hearing other opinions on this.
 
---

https://docs.djangoproject.com/en/dev/ref/django-admin/#django-admin-check

{{{
...

--tag <tagname>

The system check framework performs many different types of checks.
}}}

Which tags are available from django?

This is something I'm hoping to add to documentation in the near future.
 
How can I list the available tags (django tags and my tags)?

At present, there isn't any way. However, this is an interesting feature request - adding a --listtags option to `manage.py check` that just lists the tags that are available sounds like a good idea to me.
 
How can I list the available checks?

This is another area that where I'm hoping to improve documentation in the future. An automated --listchecks option would be a bit harder, because individual checks aren't registered anywhere centrally.
 
If a check needs two minutes, it is not wise to run it on "runserver"
on development machines. Is there a way to define which tests should
be included or excluded depending on the context. For example: if
"runserver" gets called don't call tests with have tag "foo".

I accept the problem in theory, but I'm having difficulty thinking of a situation where a check would take 2 minutes. My inclination is that this would be "solved by the market" - If Django (or a third party app) were to add a plugin that had extremely slow performance in real-world conditions, people would demand that it was removed or refactored. 

Do you have a specific idea for a check that might take a long time to run?
 
----

https://docs.djangoproject.com/en/dev/ref/checks/#registering-and-labeling-checks

{{{
@register('compatibility')
}}}

I don't like strings like this, since it can contain typos.

Are there objects for common tags? Example:

{{{
@register(Tag.COMPATIBILITY)
}}}

Not at present; however, that sounds like a reasonable idea.
 
Thank you for this check framework. I like it.

Thanks for the review notes! You've made a couple of really good suggestions here.

Yours,
Russ Magee %-) 

Carl Meyer

unread,
Feb 5, 2014, 6:31:43 PM2/5/14
to django-d...@googlegroups.com
On 02/05/2014 04:14 PM, Russell Keith-Magee wrote:
> On Wed, Feb 5, 2014 at 10:13 PM, Thomas Güttler <h...@tbz-pariv.de
> <mailto:h...@tbz-pariv.de>> wrote:
>
> Hi,
>
> here is some feedback to the check framework, documented at:
>
> https://docs.djangoproject.__com/en/dev/ref/checks/
> <https://docs.djangoproject.com/en/dev/ref/checks/>
>
> ---
>
>
> {{{
> Error('error message', None) # Good
> }}}
>
> I don't think this is "good". The "hint" should be optional. The
> above line
> is ugly, since you don't know why there is second argument (None).
>
>
> This was intended as a case of enforcing explicit > implicit. The next
> line in the documentation reads:
>
> {{{
> Error('error message', hint=None) # Better
> }}}
>
> suggesting that the preferred syntax is to explicitly state that there
> isn't a hint.
>
> However, I can see how this might be slightly contrary to expectation
> for Python developers generally. I'd be interested in hearing other
> opinions on this.

I don't think there's a reason to require the argument; if `None` is a
valid value for the argument, then clearly it is semantically optional,
so give it a default of `None` and make it actually optional.

Carl

Florian Apolloner

unread,
Feb 5, 2014, 7:04:30 PM2/5/14
to django-d...@googlegroups.com


On Thursday, February 6, 2014 12:31:43 AM UTC+1, Carl Meyer wrote:
> However, I can see how this might be slightly contrary to expectation
> for Python developers generally. I'd be interested in hearing other
> opinions on this.

I don't think there's a reason to require the argument; if `None` is a
valid value for the argument, then clearly it is semantically optional,
so give it a default of `None` and make it actually optional.

+1, Either make it completely required (eg don't allow None) or let None be the default.

Curtis Maloney

unread,
Feb 5, 2014, 8:35:20 PM2/5/14
to django-d...@googlegroups.com
I disagree.

If a hint really ought be provided, but can be omitted in the right circumstances, it should be a required value.

This will encourage people to provide a hint, or deliberately omit one.

Seems more Explicit and Implicit to me... explicit intention to omit a hint?

--
Curtis



--
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-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/2880d3fe-10a9-4270-830a-0e362fea0e09%40googlegroups.com.

For more options, visit https://groups.google.com/groups/opt_out.

Russell Keith-Magee

unread,
Feb 5, 2014, 9:43:08 PM2/5/14
to Django Developers
Hi Curtis,

The catch is that a hint isn't always possible - or the only reasonable hint would be redundant.

For example:

ERROR: max_length exceeds 100.
    HINT: Make max_length less than 100.

The hint doesn't actually clarify anything above what the error message already tells you. You can see this in the 1.6 validation framework - In the case where the connection between the problem and the solution aren't obvious (e.g., adding related_name to avoid a collision), a hint is provided, but in other cases, the problem points directly at the solution.

So - hint isn't strictly *required* - it's just highly recommended when appropriate.

So - how to implement this in Python? If we could enforce the use of keyword arguments, then the syntax would always read "Error('message', hint=None)", which is explicit; the problem is that we can't really do that; we have to allow hint as the second argument, which leaves the door open to "Error('message', None)" - syntactically legal, but not exactly expected.

The only way I can see to implement this "properly" is to have Error() accept **kwargs, and do the argument processing ourselves (i.e., specifically look for the hint kwarg). While this is possible, it's more messy than the alternative, IMHO.

Russ %-)

Tomáš Ehrlich

unread,
Mar 31, 2014, 8:39:08 AM3/31/14
to django-d...@googlegroups.com
Hi,
I've added patches to both mentioned tickets:


Review would be appreciated.

Cheers,
   Tom


Dne středa, 5. února 2014 15:13:25 UTC+1 guettli napsal(a):
Reply all
Reply to author
Forward
0 new messages