Opened 9 months ago

Closed 8 months ago

Last modified 8 months ago

#22857 closed New feature (fixed)

Django 1.7 runserver auto-reload : setting behaviour by extensions

Reported by: artscoop Owned by: nobody
Component: Core (Management commands) Version: 1.7-beta-2
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi,
I am experiencing a problem introduced by Django 1.7b2 in django-rosetta (app for editing locale files).
In Django 1.6, in a development server, rosetta responded to a button click, updated .po files and redirected to the same page.
In Django 1.7, still in a development server, rosetta responds to a button click, updates .po files and the server is immediately killed and restarted (connection closed). This is an unwanted behaviour and I have no control over this.

Is there a way to tell runserver's autoreload mechanism to ignore locale files, or file extensions ?
Running runserver with the --noreload flag is not really an option.

Change History (18)

comment:1 Changed 9 months ago by timo

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

I guess this is #9523. One user's feature is another user's bug... I guess it's likely possible to add a flag to control the behavior although I'm not sure it's ideal to be adding more flags like that to runserver.

comment:2 Changed 9 months ago by artscoop

In my opinion, the request in #9523 was a very valid one and still is... but yes, some django apps help with editing translations. This is one category of apps that defeats the purpose of autoreloading on .mo changes. :(

comment:3 Changed 9 months ago by andrewgodwin

Not entirely sure this is a release blocker, though - it'd be nice to fix, but at the same time I don't think it's a regression from previously documented behaviour, just a pattern that happened to work.

Said app could, for example, use an ajax request to kick off the rebuild and then poll the server to see when it reappears before reloading.

(Not saying we shouldn't land a fix for this in 1.7, just saying that I'm not really prepared to hold the entire release up for it...)

comment:4 Changed 9 months ago by CollinAnderson

would #18855 solve the problem?

comment:5 Changed 9 months ago by timo

I am not sure if it will, but it currently has some issues and even if we resolve them, I'm not sure backporting to 1.7 this late in the release cycle is acceptable. Currently the "best" solution I am thinking of is adding a --noreload-translations flag to runserver. Open to other opinions and ideas!

comment:6 Changed 9 months ago by claudep

Another idea (completely untested) would be to gather mo files in another set of files in runserver and only reset translations when one of these files changes instead of restarting the server.

comment:7 Changed 9 months ago by artscoop

@claudep I didn't know it was possible to reset/reload translations separately (even though it makes perfect sense), but I think it's better than the --noreload-translations, because the command option would have to be officially documented, and maybe deprecated very quickly.
The suggestion about AJAXing a view until the server comes back is good but a bit hacky.

I do think resetting translation cache on .mo change is the best idea, and would be more immediate than reloading the whole dev server (which ideally should be reloaded on code changes only). It seems that resetting the translation is a simple as in this example: https://djangosnippets.org/snippets/1704/

comment:8 Changed 9 months ago by andrewgodwin

If we can reset the translation cache then let's try that, but I'm really pushed to define this as a release blocker (it is not breaking any part of Django directly, and the behaviour it produces can be worked around with --noreload if you need to file translations), so if we get down to the release time and this is what's left... it might not make it.

comment:9 Changed 9 months ago by aaugustin

  • Severity changed from Release blocker to Normal

I agree that this change of behaviour is annoying for rosetta but I don't think we should delay 1.7 for this edge case.

comment:11 Changed 8 months ago by timo

  • Triage Stage changed from Accepted to Ready for checkin

Patch seems okay to me, although I'm not really familiar with this stuff so a second pair of eyes wouldn't hurt.

comment:12 Changed 8 months ago by claudep

I'd like at least the reporter to review and check the patch.

comment:13 Changed 8 months ago by artscoop

To be done in the next 8 hours, I'll keep you informed.

comment:14 Changed 8 months ago by artscoop

@claudep Works flawlessly. Could not test on the master branch since it breaks many third party apps, but copied the autoreload.py file into the working 1.7b4. the server did not restart on .mo file changes, yet the new translation was properly taken into account. Of course it restarted properly on .py code changes.

Last edited 8 months ago by artscoop (previous) (diff)

comment:15 Changed 8 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 0d363b25b82b0a86b7243512470f364bef92bc3d:

Fixed #22857 -- Reset translations when only .mo file changed

No need to restart the server when a translation file changes.
Refs #9523. Thanks artscoop for the report and Tim Graham for
the review.

comment:16 Changed 8 months ago by claudep

@artscoop Thanks for the feedback!
I won't make alone the decision to backport to 1.7. If other core devs think it's a good idea, just speak.

comment:17 Changed 8 months ago by timo

I would backport this or revert #9523 from 1.7 so we don't ship a buggy change.

comment:18 Changed 8 months ago by Tim Graham <timograham@…>

In cbcb7c010bdc015c79b4b8a0ba2955abb23aee82:

[1.7.x] Fixed #22857 -- Reset translations when only .mo file changed

No need to restart the server when a translation file changes.
Refs #9523. Thanks artscoop for the report and Tim Graham for
the review.

Backport of 0d363b25b8 from master

Note: See TracTickets for help on using tickets.
Back to Top