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

Add server side validation, new auto-reload method, and save po files with FileLock #299

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

balazs-endresz
Copy link
Contributor

All Submissions:

  • Are tests passing? (From the root-level of the repository please run pip install tox && tox)
  • I have added or updated a test to cover the changes proposed in this Pull Request
  • I have updated the documentation to cover the changes proposed in this Pull Request

This is based on #296, which needs to be merged first but I thought I might as well add a new PR already.

This adds the following:

  • ROSETTA_VALIDATE: Server side validation that shows the same errors that compilemessages does. But this includes errors in fuzzy entries as well, because I guess there's no reason not to review those asap too.
  • ROSETTA_AUTO_RELOAD: New auto-reload option to make the translation changes automatically visible without restarting anything (see docs for details).
  • Not just AUTO_RELOAD but now also AUTO_COMPILE requires that validation should pass. Except if ROSETTA_VALIDATE is disabled then these are allowed to go ahead regardless, mainly for backwards compatibility and just to give users more control.
  • Use filelock to prevent multiple processes from writing to the same file in parallel and corrupting it. It raises 500 error if it can't acquire a lock in 5 seconds, which should be highly unlikely. Even then simply reloading the page will re-submit the edits. This is on top of the existing mechanism that prevents parallel edits, which I think can theoretically allow two edits at the exact same time and then both processes might write concurrently to the same files. This should give more confidence that the po and mo files won't be corrupted, even though I guess this issue would be very rare.
  • I've grouped together some existing options in the docs, and changed description for options in the 'Storages' section.
  • Javascript validation errors are now shown on page load too, not just after making changes to an entry. This should also help finding validation errors that need to be fixed for the auto-reload.
  • Added a few logger.exception()s for errors during save/reload.

Notes:

  • compilemessages is a bit broken: https://code.djangoproject.com/ticket/36010 - In case you'd want to compare the output then currently you need to touch the po file first: that gets around this bug.

  • I haven't tested it yet but looks like the handling of f-strings is changing: https://code.djangoproject.com/ticket/35993 So with newer versions of gettext I guess it might start showing errors related to these too.

  • To test auto-reload I've used another private project that already had nginx and gunicorn set up, which is very close to what many production environments look like too.

    • pip uninstall django-rosetta -y && pip install git+https://github.com/balazs-endresz/django-rosetta.git@extend-django-admin-validate-auto-reload#egg=django-rosetta
    • gunicorn options:
      • workers = 4
      • reload = False
    • django settings:
      • DEBUG = False # to prevent django from auto reloading of mo files
      • ROSETTA_VALIDATE = True
      • ROSETTA_AUTO_COMPILE = True
      • ROSETTA_AUTO_RELOAD = True
      • Added 'rosetta.middleware.AutoReloadMiddleware' to settings.MIDDLEWARE
    • Added process id to template context: import os; os.getpid()
    • Loaded multiple pages to warm up the django translations cache. Checking that it hits all processes.
    • Modified a translation in rosetta.
    • Checked the same pages again, reloading them multiple times to hit all processes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant