Differential D2826 Diff 9224 pillar/cli/translations.py

Changeset View

Standalone View

pillar/cli/translations.py

This file was added.

				import logging

				import os
				sybrenUnsubmitted Not Done Inline Actions Logging isn't used, so better to remove the import. sybren: Logging isn't used, so better to remove the import.
				import pathlib
				import subprocess

				sybrenUnsubmitted Not Done Inline Actions Keep your imports of Python's stdlib together, without newlines, ordered alphabetically. sybren: Keep your imports of Python's stdlib together, without newlines, ordered alphabetically.
				from flask_script import Manager

				from pillar import current_app

				log = logging.getLogger(__name__)

				manager_translations = Manager(
				current_app, usage="Translation scripts, to create, update and compile localization")


				def get_application_folder() -> pathlib.Path:
				"""
				Folder to look for strings to translate
				"""
				return pathlib.Path(".")
				sybrenUnsubmitted Not Done Inline Actions Why does this have to be a function? And why can it be relative? What is it relative to? Does it still work when the translation commands are not given from the top level directory of the project? sybren: Why does this have to be a function? And why can it be relative? What is it relative to? Does…
				sybrenUnsubmitted Not Done Inline Actions Use single quotes when possible. sybren: Use single quotes when possible.
				sybrenUnsubmitted Not Done Inline Actions Does this really return the pathlib module? sybren: Does this really return the pathlib module?


				def get_translations_folder() -> pathlib.Path:
				"""
				Returns folder where we store the translations
				"""
				return get_application_folder().joinpath('translations')


				def get_babel_settings() -> dict:
				"""
				Return a dict with all the required settings
				sybrenUnsubmitted Not Done Inline Actions What should happen when an exception is raised? Shouldn't that also clean up messages.pot? According to the docstring it is, but the code doesn't do this. sybren: What should happen when an exception is raised? Shouldn't that also clean up messages.pot?
				for the translations commands
				"""
				ret_dict = {
				sybrenUnsubmitted Done Inline Actions I'm not too keen on using a dict for this. It makes it hard to figure out which keys there are, and half of the dict contents are hardcoded anyway, so could be module-level constants. The only two non-hardcoded values will in the end be turned into strings `'.'` and `'./translations'`. It seems unclear to me why a new dict has to be constructed and three functions called, just to get these hardcoded settings that won't change between invocations. sybren: I'm not too keen on using a dict for this. It makes it hard to figure out which keys there are…
				'babel_config': 'translations.cfg',
				'messages_pot': 'messages.pot',
				'application_folder': get_application_folder(),
				'translations_folder': get_translations_folder(),
				}
				return ret_dict


				@manager_translations.command
				sybrenUnsubmitted Not Done Inline Actions The use of `create_messages_pot()` seems to consistently be create; run other command; cleanup. If the cleanup should always run, even when the "other command" fails, it would be better to turn `create_messages_pot()` into a context manager using `@contextlib.contextmanager`. sybren: The use of `create_messages_pot()` seems to consistently be create; run other command; cleanup.
				def init(locale):
				"""
				Initialize the translations for a new language.
				"""
				babel_settings = get_babel_settings()

				subprocess.call('pybabel extract -F {babel_config} -k lazy_gettext -o {messages_pot} {application_folder}'.format(
				sybrenUnsubmitted Done Inline Actions Don't pass the entire command as a string, this is very fragile and a potential security flaw. Use a list of individual arguments instead, and don't rely on `str.split()` to do this for you. For example, a slight change in `get_application_folder()` could result in an absolute path being used, which could contain spaces, breaking this command. sybren: Don't pass the entire command as a string, this is very fragile and a potential security flaw.
				sybrenUnsubmitted Not Done Inline Actions Don't use `subprocess.call()`, as it doesn't check the subprocess' return value. Use `subprocess.run()` instead, as this is [the recommended way to use the `subprocess` module](https://docs.python.org/3/library/subprocess.html#using-the-subprocess-module). sybren: Don't use `subprocess.call()`, as it doesn't check the subprocess' return value. Use…
				**babel_settings).split(' '))

				subprocess.call('pybabel init -i {messages_pot} -d {translations_folder} -l {locale}'.format(
				**babel_settings, locale=locale).split(' '))

				pathlib.Path('{messages_pot}'.format(
				sybrenUnsubmitted Done Inline Actions Instead of `'{messages_pot}'.format(babel_settings)`, just use `babel_settings['messages_pot']`. It's more readable, more explicit, and faster to execute. sybren: Instead of `'{messages_pot}'.format(babel_settings)`, just use `babel_settings…
				**babel_settings)).unlink()


				@manager_translations.command
				def update():
				"""
				Update the strings to be translated.
				"""
				babel_settings = get_babel_settings()

				subprocess.call('pybabel extract -F {babel_config} -k lazy_gettext -o {messages_pot} {application_folder}'.format(
				sybrenUnsubmitted Done Inline Actions This is the exact same invocation as from the function above, so put it into its own Python function. sybren: This is the exact same invocation as from the function above, so put it into its own Python…
				**babel_settings).split(' '))

				subprocess.call('pybabel update -i {messages_pot} -d {translations_folder}'.format(
				**babel_settings).split(' '))

				pathlib.Path('{messages_pot}'.format(
				**babel_settings)).unlink()
				sybrenUnsubmitted Not Done Inline Actions It's clear that this is a list; `available_modes` would be a better name. Or even easier, just use the list literal directly at the `choices=...` argument, since it's used only once anyway. sybren: It's clear that this is a list; `available_modes` would be a better name. Or even easier, just…


				@manager_translations.command
				def compile():
				"""
				Compile the translation to be used.
				"""
				babel_settings = get_babel_settings()

				subprocess.call('pybabel compile -d {translations_folder}'.format(
				**babel_settings).split(' '))


				def parse_arguments():
				sybrenUnsubmitted Not Done Inline Actions Declare the return type sybren: Declare the return type
				"""
				When calling this script directly we need to handle the arguments
				as if we were a Pillar extension running via Flask cli
				"""
				import argparse
				list_of_modes = ['init', 'update', 'compile']

				parser = argparse.ArgumentParser(description='Translate Pillar')

				parser.add_argument(
				'mode',
				type=str,
				help='Init once, update often, compile before deploying.',
				choices=list_of_modes)

				parser.add_argument(
				'languages',
				nargs='*',
				type=str,
				help='Languages to initialize: pt it es ...')

				args = parser.parse_args()
				if args.mode == 'init' and not args.languages:
				parser.error("init requires languages")

				return args


				def main():
				"""
				For Pillar we need to run this via command-line, without relying on Flask-cli
				sybrenUnsubmitted Not Done Inline Actions What does "For Pillar" mean? sybren: What does "For Pillar" mean?
				So we parse the arguments, and init/update/compile the translations strings
				"""
				args = vars(parse_arguments())
				sybrenUnsubmitted Done Inline Actions Why do you need a `vars()` call here? Just use `args.mode` and `args.languages` directly. sybren: Why do you need a `vars()` call here? Just use `args.mode` and `args.languages` directly.
				mode = args['mode']

				if mode == 'init':
				for language in args['languages']:
				init(language)

				elif mode == 'update':
				update()

				else: # mode == 'compile'
				compile()


				if __name__ == '__main__':
				main()