GSoC 2014 proposal [Improving error reporting]

[anubhav joshi]

Hi all,
        I had submitted a proposal earlier also related to aggregation/annotation but since almost major things had been implemented by him, I am thinking to switch to 'improving error reporting'.

I am going through the errors mentioned. I have some questions for now.

1.) The two problematic errors related to forms have been fixed by small patches, so I would like to know if there are any other similar cases.

2.) There it says: "Import errors discovered during application loading during can be masked under certain circumstances."
If I could get a better explanation of this, it would be very helpful.

3.) There are some tickets marked as 'wontfix' so I want to know whether I am being expected to somehow fix those if possible.

Thanks,

Anubhav Joshi
IIT Patna

Github: https://github.com/coder9042

[Tim Graham]

A starting place for this would be to bring the BetterErrorMessages wiki page up-to-date by removing items that have been fixed or "won't fixed". Then it'll be easier to see what's left and whether or not there is enough to fill 12 weeks.

[anubhav joshi]

On Monday, February 24, 2014 9:45:30 PM UTC+5:30, Tim Graham wrote:

A starting place for this would be to bring the BetterErrorMessages wiki page up-to-date by removing items that have been fixed or "won't fixed". Then it'll be easier to see what's left and whether or not there is enough to fill 12 weeks.

I have done some work and have updated the wiki: https://code.djangoproject.com/wiki/BetterErrorMessages

The following have been added:

django.contrib.auth
https://code.djangoproject.com/ticket/18959

django.core
https://code.djangoproject.com/ticket/22055
https://code.djangoproject.com/ticket/22058
https://code.djangoproject.com/ticket/21668

django.core.urlresolvers
https://code.djangoproject.com/ticket/8809

django.db
https://code.djangoproject.com/ticket/20752
https://code.djangoproject.com/ticket/21430
https://code.djangoproject.com/ticket/10811
https://code.djangoproject.com/ticket/7074

django.forms
https://code.djangoproject.com/ticket/15069
https://code.djangoproject.com/ticket/13091

django.template
https://code.djangoproject.com/ticket/16383

Now I would like to know whether it would be suitable for GSoC?

Regards,
Anubhav Joshi
IIT Patna

[anubhav joshi]

Also there were many links to those tickets which were closed because either they were "fixed" or they "wontfix". So I have removed those as they were unnecessary on that wiki.

2.) There it says: "Import errors discovered during application loading during can be masked under certain circumstances."
If I could get a better explanation of this, it would be very helpful.

Also If I could get an answer to the above.

[Aymeric Augustin]

On 24 févr. 2014, at 21:52, anubhav joshi <anubh...@gmail.com> wrote:

2.) There it says: "Import errors discovered during application loading during can be masked under certain circumstances."
If I could get a better explanation of this, it would be very helpful.

Also If I could get an answer to the above.

Overall, this happens when Django catches an exception and re-raises another exception, usually ImproperlyConfigured, in order to provide a “more helpful” error message. This gets nasty when the original exception is already hard to diagnose with the full context, for instance when it’s an ImportError triggered by a circular import. It’s also quite bad when another part of Django catches ImproperlyConfigured again. I’ve seen that, but I cannot remember where.

There’s a particularly vicious variant of the circular import problem: when it’s triggered by a reentrant call to the app cache population process. Attempts to remove the faulty circular import may change the import sequence and create another circular imports, making for a particularly annoying whack-a-mole game. This is fixed in 1.7. Have a look at my posts discussing circular imports in the context of app-loading in December and early January for details.

-- 

Aymeric.

[Tim Graham]

"Now I would like to know whether it would be suitable for GSoC?"

Maybe! As these tickets touch many different aspects of Django, I'm guessing you'll need to do quite a bit of research in order to convince us you have the breadth of experience to tackle them all. Some of those tickets also have existing patches -- you'll need to review them to see if additional work needs to be done (or if not, please bump the ticket to ready for checkin). If I were you, I'd map out more than 12 weeks worth of tickets in case some of the ones you choose initially are completed by community members in the meantime. Presumably if your proposal were accepted, you could assign them to yourself at that time.

[anubhav joshi]

On Tuesday, February 25, 2014 5:02:26 AM UTC+5:30, Tim Graham wrote:

"Now I would like to know whether it would be suitable for GSoC?"

Maybe! As these tickets touch many different aspects of Django, I'm guessing you'll need to do quite a bit of research in order to convince us you have the breadth of experience to tackle them all.

Yes I have already started looking into those issues and will be posting soon as and when I reach somewhere.
 

Some of those tickets also have existing patches -- you'll need to review them to see if additional work needs to be done (or if not, please bump the ticket to ready for checkin). If I were you, I'd map out more than 12 weeks worth of tickets in case some of the ones you choose initially are completed by community members in the meantime. Presumably if your proposal were accepted, you could assign them to yourself at that time.

Ok will keep that in mind

[anubhav joshi]

On Tuesday, February 25, 2014 5:02:26 AM UTC+5:30, Tim Graham wrote:

"Now I would like to know whether it would be suitable for GSoC?"

Maybe! As these tickets touch many different aspects of Django, I'm guessing you'll need to do quite a bit of research in order to convince us you have the breadth of experience to tackle them all. Some of those tickets also have existing patches -- you'll need to review them to see if additional work needs to be done (or if not, please bump the ticket to ready for checkin). If I were you, I'd map out more than 12 weeks worth of tickets in case some of the ones you choose initially are completed by community members in the meantime. Presumably if your proposal were accepted, you could assign them to yourself at that time.

In order to chalk out work enough for 12 weeks, I think I will extend my proposal to 'improving error reporting and fixing related bugs/issues.' because while I am going through the issues, I have found many tickets which need work to be done regarding some strange thing happening, but they are not related to 'improving error reporting' but are worth fixing and AAMOF they interest me as well.


Regards,
Anubhav Joshi

[anubhav joshi]

Regarding the 'improving error reporting' part, I have started with some issues which may be comparatively easier:

Improving error reporting

django.forms

#15069

In forms.py:
    self._errors[name] = self.error_class(e.messages)

When the list of errors is created, the value of the wrong/invalid field can be shown.

Some extra information could be shown, based upon the type of error.
eg. In FileField : In case of invalid, the encoding-type problem could be explained with better message/suggestion
    In DateTimeField : Format/Example of correct datetime object or errors could be pointed out more explicitly.
Also such cases in other Field subclasses

Also some more type of errors could be added.

#13091

A patch was written for a part of this(which was decided to be dealt in another ticket) which was merged. But the major problem remained unsolved. Later a patch was written for this too, but as summarized by a core-dev, that will lead to another set of bugs.
I did go through the patch and realized the situation, there were some suggesstions that I too believe will work well:

Just fix this in the particular case of the admin. This could possibly be done by adding a manual call to full_clean once the instance is fully populated (might be a bit ugly getting any ValidationErrors back into the not-valid code path).
Add a warning to the above section of the docs (on partial ModelForms) about this issue and how to work around it in your own forms.


django.http

If Http404 is returned instead of raised(because it is in the same file where other response classes have been defined i.e. django/http/response.py), then there is a problem of AttributeError.
One solution is renaming it to more clear name like Http404Exception, but we would have to replace it at several places.
Ithink a better solution could be to check this error in django/core/handlers/base.py .We can check the called response object for this, and then do something which is appropriate. I think we can check for Http404, if it is returned and convert the response to HttpResponseNotFound.


Feedback Needed.
Working on other issues, will be posting soon.

[anubhav joshi]

Here is some more analysis:

django.db


1.)
There is a problem with ForeignKey, which when given only blank=True causes ValidationError as they do not allow null values. If blank=True, null=True is specified then, there is no problem.

The operation of ModelForms is such so as to construct instances at validation time, not at save-time, Null and wrong-model assignments to ForeignKey fields are disallowed prior to save-time. Therefore, there is problem when we try to call is_valid() or save().
Currently if blank=True and null is not True, ValueError is raised. The check being in related.py #399 for ForeignKey.
a.) I think we can display a correct message depicting the problem that in ForeignKey with null=True can only be left blank.
b.) We can stop the usage of blank=True with ForeignKey, because alone without null=True defined it has no significance and give suitable error message instead of raising an error. null=True alone works perfectly
Related: #13776


2.)
Unpickling Models and QuerySet from incompatible version.

A patch was submitted in regard for this, but it breaks the tests. So, some work needs to be done to fix that.Probably it s because of some fault in django/utils/version.py as far as I have apprehended.
Also 'akaariai on github' suggested that we can think of extending this behaviour to do cross-version conversions in case of model unpickling. Upon doing some study and having a discussion on irc with 'apollo13', I have understood how the pickling works and what protocol does django uses. Further I have understood that to do the required, we would need to convert internal data-structures between versions. This could be achived by first finding out the version of the current installation and pickled data. We can store the differences in _meta for models for different versions in a separate file e.g pickle_ver.py. Then accordingly we can define __setstate__() on its basis. We can create a new model instance from the unpickled data, using pickle_ver.py,  and then use its state for __dict__.
Related: #21430


3.)
In certain situations with PostgreSQL, a bogus error message about SET TIME ZONE may be returned. See #3179 (which is closed, but has a description of the problem). The real error message can probably be found in the postgres log file.
Till now I have only understood what the problem is. I would need some guidance here, coz I have no knowledge of PostgreSQL as of now. I myself am also trying to consult someone whom I know for this.
Although as mentioned for the shell part, we can do the same for views also, we need to figure out how to catch that previous error(due to which we get this behaviour) and then perform cursor rollback with appropriate warning.


4.)
#10811

I had written a patch for this but it got stuck at a place leading to failure of two tests. I didn't got the time to look into it much then,
I will do that and sort this out.
https://github.com/django/django/pull/2162

5.)
There has been strange errors being reported because of lack of date[time] CAST conversions in backends.
This has been extensively discussed in #7074.
Only Oracle implements this correctly. A sample workaround as suggested in the ticket:

django/db/models/sql/where.py in make_atom

def datetime_cast_sql(self):
    return 'CAST(%s AS DATETIME)'

def rhs_cast_sql(self, db_type):
    if db_type in ['date', 'datetime']:
        return 'CAST(%%s AS %s)' % db_type
    return '%s'

In where.py make_atom

if value_annot is datetime.datetime:
    cast_sql = connection.ops.datetime_cast_sql()
else
    cast_sql = connection.ops.rhs_cast_sql(db_type)

But the thing is that, we are putting the fix in two different places.
What is required here is a single general right hand casting hook so that when values when are
passed as strings instead of datetime.datetime objects then we can CAST them appropriately for different backends.

For this I am not sure whether this CASTING should be done only for date[time] objects or it might be required in some other cases as well.
I need some review here. Only then can I proceed towards writing that single casting function.

[anubhav joshi]

Please have a look at all the issues mentioned, correct me where I am wrong and there are places where I need help(I have mentioned in those places.)
I need feedback and reviews.
Will be posting back again very soon with further analysis of other issues.

[anubhav joshi]

Please give me any reviews and suggestions regarding the issues, as and when I am posting about them.
So that when I my studying and analysis of the issues is done, then I can chalk out a proper timeline for the required work as well.

[Russell Keith-Magee]

Hi Anubhav,

Please keep in mind that this is a global mailing list, and if you want considered feedback, it's going to take time. Posting two "Please give me feedback" pings in 6 hours isn't especially helpful. There will be people who have been asleep for that entire period.

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-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/1ca9ce43-5b59-4601-b6d2-5be43b28d740%40googlegroups.com.

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

[anubhav joshi]

On Wednesday, February 26, 2014 7:43:49 PM UTC-8, Russell Keith-Magee wrote:

Hi Anubhav,

Please keep in mind that this is a global mailing list, and if you want considered feedback, it's going to take time. Posting two "Please give me feedback" pings in 6 hours isn't especially helpful. There will be people who have been asleep for that entire period.

Yours,

Russ Magee %-)

Apologies for being over eager.

[anubhav joshi]

Here is another issue that I have analysed.

When the autogenerated field name is too long, there is no proper displaying of errors, many a time silent failures for some database.
Written a fix, although some thinking is required in writing the tests.

@classmethod
def _check_long_column_name(cls):
    errors = []
    allowed_len = None
    db_alias = None

    for db in settings.DATABASES.keys():
        # skip databases where the model won't be created
        if not router.allow_migrate(db, cls):
            continue
        connection = connections[db]
        allowed_len = connection.ops.max_name_length()
        db_alias = db

    if allowed_len is not None:
        for f in cls._meta.local_fields:
            _, column_name = f.get_attname_column()
            """
            Check if auto-generated name for the field is too long
            for the database.
            """
            if f.db_column is None:
                if len(column_name) > allowed_len:
                    errors.append(
                        checks.Error(
                            'Autogenerated column name too long for field "%s".'
                            'Maximum length is "%s" for "%s".' % (column_name, allowed_len, db_alias),
                            hint='Set the column name manually using db_column',
                            obj=cls,
                        )
                    )

    return errors

Also it is seen, that model name truncation is not done properly.
self.db_table = truncate_name(self.db_table, connection.ops.max_name_length()) is done.

But, something like the following would be more correct:

db = router.db_for_read(self.model)
self.db_table = truncate_name(self.db_table, connections[db].ops.max_name_length())

We could also use router.allow_migrate as above.

Also when db_table name truncation is done, we get a weird message:

#models.py
class aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(models.Model):
    name = models.CharField(max_length=40)

python manage.py syncdb

Creating table probs_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa36a3
Warning: Data truncated for column 'name' at row 13

Instead of reporting about the truncated name of the model table, it seems to tell about the first field of that table.
This is something done by the backend I believe.(Correct me if I am wrong.)
Assuming if above is correct, then what we can do is create a warning using warning.warn(...) about the correct issue.
Then we can remove the false warning arising from the db.

A mention of similar problem at #18959 is there, but as far as I have understood it worksforme or else if someone can help me reproduce the problem/error if I am wrong, then it would be very helpful.

[Tim Graham]

I think it'll be better to put your analysis of each ticket on the ticket itself. Then when you're finished with that, put together a more high level overview of the analysis you've done. I think it will be easier to give feedback if you structure your thoughts in this way. Thanks!

[anubhav joshi]

On Friday, February 28, 2014 2:03:10 AM UTC+5:30, Tim Graham wrote:

I think it'll be better to put your analysis of each ticket on the ticket itself. Then when you're finished with that, put together a more high level overview of the analysis you've done. I think it will be easier to give feedback if you structure your thoughts in this way. Thanks!

If I put my analysis on all the tickets that I have studied and analysed, and then if someone on studying that analysis, fixes the issue then what will I put in my proposal? I mean I have to ensure that I have sufficient work for GSoC. How do I that ?

[Tim Graham]

I believe if your tickets are enough work to fill 12 weeks, someone is not going to come along and complete a majority of them in the meantime. You can also add something like "I hope to work on this ticket for GSoC 2014."

[anubhav joshi]

As suggested by Tim Graham, I am going to post my analysis on respective tickets.
But there are some issues where I need help in analysing, I have mentioned those in my above posts. Also there may be issues for which ticket have not been opened, they are only mentioned in the wiki page.
Hence, it is my humble request to please go through the issues here as well once for the above mentioned reasons.