Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #30323 -- Fixed spurious autoreload failures. #11177

Merged
merged 3 commits into from Apr 29, 2019
Merged

Fixed #30323 -- Fixed spurious autoreload failures. #11177

merged 3 commits into from Apr 29, 2019

Conversation

orf
Copy link
Sponsor Contributor

@orf orf commented Apr 5, 2019

Ticket: https://code.djangoproject.com/ticket/30323

A few fixes and changes here:

  1. Loading the urlconf module can result in SyntaxErrors. We now swallow them to prevent the reloader from quitting.

  2. Simplified the ensure_echo_on method

  3. Simplified the stat autoreloader. The implementation is now pretty much exactly the same as the one in Flask/Werkzeug

  4. The thread running Django itself is now named django-main-thread to help with tracebacks.

@carltongibson
Copy link
Member

Hey @orf. Thanks for your efforts here. Is it worth you combining fixes into a single PR here (separate commits if you want) whilst you're working on it? (I don't mind: what do you think?)

@orf
Copy link
Sponsor Contributor Author

orf commented Apr 5, 2019

Sure thing, if that's easier to review. I'll bundle them all into this PR 👍

@carltongibson
Copy link
Member

Works for me. Let me know if I can help at all testing and such. 👍

@orf
Copy link
Sponsor Contributor Author

orf commented Apr 6, 2019

I've made a few fixes here. When I originally wrote the StatReloader I wanted it to be able to pick up newly created files (that have been watched but do not exist yet, like translation files) as well as files that have been modified.

I did this by tracking the time.time() of the last loop and comparing it to any new files we have picked up. As far as I can tell on some Windows platforms and in some situations this caused issues which I cannot quite explain.

So I've just simplified the stat reloader to be pretty much identical to the one in Werkzeug. I will see if I can find anyone to test this (I've pushed it to a fix-stat-reloader branch on my fork, which is right off 2.2).

@orf orf changed the title Refs #30323 -- Swallow errors while loading the urlconf module Fixed #30323 -- Fix spurious autoreload failures Apr 6, 2019
@carltongibson
Copy link
Member

Hey @orf: anything your end still to do here? (Thanks!)

@orf
Copy link
Sponsor Contributor Author

orf commented Apr 25, 2019

Not on this PR, I think this is complete and ready to go. However there is #11263 which I would say should be included as well if possible, even though it’s not marked as a release blocker.

Copy link
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orf Thanks for this patch 👍 I left two comments.

docs/releases/2.2.1.txt Outdated Show resolved Hide resolved
django/utils/autoreload.py Show resolved Hide resolved
@felixxm felixxm self-assigned this Apr 29, 2019
@felixxm felixxm changed the title Fixed #30323 -- Fix spurious autoreload failures Fixed #30323 -- Fixed spurious autoreload failures. Apr 29, 2019
@felixxm felixxm merged commit 6754bff into django:master Apr 29, 2019
@orf orf deleted the fix-autoreload-urlconf-issue branch April 29, 2019 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants