Noisy errors on startup

[sondrelg]

Hi 👋

Since implementing deprecate_field we've started seeing this output whenever we initialize django:

accessing deprecated field NoneType.<unknown>
accessing deprecated field NoneType.<unknown>

After a lot of digging I have not been able to get to the bottom of exactly where our deprecated field is called, but it seems to be indirect. There are no direct calls to the deprecated field anywhere.

Would you accept a PR where we change the default behavior to only outputting this if either the class name or object name is known?

[David-Wobrock]

Hey @sondrelg

On top of my head, could it make sense to display the call stack trace? Maybe through an option, so that one can get a better understanding from where the access comes from.

[sondrelg]

That helped 🥇 Thanks!

This is the culprit: https://github.com/viewflow/django-fsm/blob/master/django_fsm/__init__.py#L436

This library method is triggered by a django signal that's fired on startup (and is supposed to AFAICT), from here: https://github.com/django/django/blob/main/django/db/models/base.py#L428

[David-Wobrock]

That's indeed hard to catch 🙈

Should django-deprecate-fields display the stack trace? Maybe when verbosity is high enough 🤔

[sondrelg]

I was thinking more along the lines of just not outputting the warning in these cases. The purpose of the warning is to avoid calls to deprecated fields right? This isn't that, so maybe the warning isn't necessary?

What do you think?

[David-Wobrock]

The purpose of the warning is to avoid calls to deprecated fields right?

Yes indeed.

This isn't that, so maybe the warning isn't necessary?

It isn't? It's still a call to the deprecate field.
Sure, it's not the code base but a lib, but it's still a call to the field 🤔
How would we differentiate a valid call and one that can be ignored?

[sondrelg]

What I mean is, it's a call to the field, but with this type of implementation:

for field in model.__fields__:
    print(field.__name__)

This is psuedo-code, but my point would be that this would trigger the warning even though the operation is harmless (and unavoidable). So my thinking is that this would be ok not to warn about on the basis that there's no direct invocation of an unused field - it's just a mapping operation happening on a model that happens to contain a deprecated field. Do you see it differently?

[David-Wobrock]

I agree that it's probably a case that doesn't require warning.
But I wonder how the library can detect this case.

How would the library be able to know if the call if explicit on the field, or implicit because we iterate over all fields 🤔

[sondrelg]

Yeah true 🤔 The simplest fix for my case would be to inspect the stack and check for inspect.getmembers - but that's perhaps a bit janky and inefficient.

Then again, this code isn't meant to run very often, so maybe something alone those lines would be acceptable?

[hvdklauw]

Same issue as #19
As suggested there catching the None object would solve it, if the obj is None, nothing should be reported, as your are not accessing the property on an instantiated object.