#34925: refresh_from_db() will not iterate through all of the fields listed in
the
'fields' parameter.
-------------------------------------+-------------------------------------
Reporter: Andrew Cordery | Owner: Asfand
| Yar Khan
Type: Bug | Status: assigned
Component: Database layer | Version: 5.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Andrew Cordery):
I searched through the codebase with some admitedly crap regex just
looking for similar instances of for x in y: ... y.remove(x), and I found
one other:
Line 93 of django/core/management/commands/compilemessages.py:
{{{
for dirpath, dirnames, filenames in os.walk(".", topdown=True):
for dirname in dirnames:
if is_ignored_path(
os.path.normpath(os.path.join(dirpath, dirname)),
ignore_patterns
):
dirnames.remove(dirname)
elif dirname == "locale":
basedirs.append(os.path.join(dirpath, dirname))
}}}
This behavior is explicitly encouraged by python
https://docs.python.org/3/library/os.html#os.walk, however in my tests
this will still lead to the same kind of member skipping behavior. This
particular block of code would still work correctly as long as there was
never a 'locale' dirname directly after an ignored dirname in the same
path.
For example, imagine the following directory structure, where we wanted to
extract the two 'locale' dirs (./locale and ./a/locale) and ignore the 'b'
dir.
{{{
root:
a/
locale/
b/
locale/
}}}
If you were to run the following code:
{{{
import os
locale_paths=[]
for dirpath, dirnames, filenames in os.walk(".", topdown=True):
# os.walk returns dirnames in arbitrary order
https://docs.python.org/3/library/os.html#os.listdir,
# so sort the dirnames to force the skip directory 'b' to come
directly before the 'locale' directory
dirnames.sort()
print(f'Iterating through {dirpath}/{dirnames}')
for dirname in dirnames:
path = os.path.join(dirpath, dirname)
print(f'Examining {path}...', end='')
if dirname == 'b':
print(f'Ignoring {path}')
dirnames.remove(dirname)
elif dirname == 'locale':
print(f'Found {path}')
locale_paths.append(path)
else:
print('')
}}}
You would get the following output:
{{{
Iterating through ./['a', 'b', 'locale']
Examining ./a...
Examining ./b...Ignoring ./b
Iterating through ./a/['locale']
Examining ./a/locale...Found ./a/locale
Iterating through ./a/locale/[]
Iterating through ./locale/[]
In [33]: locale_paths
Out[33]: ['./a/locale']
}}}
Notice that ./locale is never 'examined' nor does it appear in the
'locale_paths' output variable. It does however get traversed as shown by
the {{{'Iterating through ./locale/[]'}}} message, which makes sense as it
was not removed from dirnames. This does mean though that if ./locale/
had had a 'locale' subdirectory in it, ''that'' directory would have been
picked up, even though the parent was missed.
Naturally, changing the line to {{{for dirname in list(dirnames)}}} also
completely resolves this issue.
Not sure any of that matters or is useful to you guys, but it was
interesting to me. Moral of the story is don't ever mutate a list we are
iterating through, even if the python docs seem to imply that you can! I
think their docs should be amended to specifically state that you should
NOT mutate it during the initial iteration, and that instead you should
iterate through a copy (ex list(dirnames)) or only mutate it once you have
examined every member of the list that you are interested in, right before
your code exits.
--
Ticket URL: <https://code.djangoproject.com/ticket/34925#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018b669a372c-5ce56cd6-8a7b-4897-99da-029fe2486a38-000000%40eu-central-1.amazonses.com.
