Page MenuHome
Differential D2826 Diff 9243 pillar/__init__.py

Changeset View

Standalone View

View Options

pillar/__init__.py

"""Pillar server.""" """Pillar server."""
import collections import collections
import contextlib import contextlib
import copy import copy
import json import json
import logging import logging
import logging.config import logging.config
import subprocess import subprocess
import tempfile import tempfile
import typing import typing
import os import os
import os.path import os.path
import pathlib
import jinja2 import jinja2
from eve import Eve from eve import Eve
import flask import flask
from flask import render_template, request from flask import g, render_template, request
from flask_babel import Babel, gettext as _
from flask.templating import TemplateNotFound from flask.templating import TemplateNotFound
import pymongo.collection import pymongo.collection
import pymongo.database import pymongo.database
from werkzeug.local import LocalProxy from werkzeug.local import LocalProxy
# Declare pillar.current_app before importing other Pillar modules. # Declare pillar.current_app before importing other Pillar modules.
def _get_current_app(): def _get_current_app():
▲ Show 20 Lines • Show All 86 Lines • ▼ Show 20 Lines def __init__(self, app_root, **kwargs):
if not self.config.get('SECRET_KEY'): if not self.config.get('SECRET_KEY'):
raise ConfigurationMissingError('SECRET_KEY configuration key is missing') raise ConfigurationMissingError('SECRET_KEY configuration key is missing')
# Configure authentication # Configure authentication
self.login_manager = auth.config_login_manager(self) self.login_manager = auth.config_login_manager(self)
self._config_caching() self._config_caching()
self._config_translations()
# Celery itself is configured after all extensions have loaded. # Celery itself is configured after all extensions have loaded.
self.celery: Celery = None self.celery: Celery = None
self.org_manager = pillar.api.organizations.OrgManager() self.org_manager = pillar.api.organizations.OrgManager()
self.before_first_request(self.setup_db_indices) self.before_first_request(self.setup_db_indices)
def _load_flask_config(self): def _load_flask_config(self):
▲ Show 20 Lines • Show All 100 Lines • ▼ Show 20 Lines def _config_encoding_backend(self):
from zencoder import Zencoder from zencoder import Zencoder
self.encoding_service_client = Zencoder(self.config['ZENCODER_API_KEY']) self.encoding_service_client = Zencoder(self.config['ZENCODER_API_KEY'])
def _config_caching(self): def _config_caching(self):
from flask_cache import Cache from flask_cache import Cache
self.cache = Cache(self) self.cache = Cache(self)
def set_languages(self, translations_folder: pathlib.Path):
"""Set the supported languages based on translations folders
English is an optional language included by default, since we will
never have a translations folder for it.
"""
self.default_locale = self.config['DEFAULT_LOCALE']
self.config['BABEL_DEFAULT_LOCALE'] = self.default_locale
# Determine available languages.
languages = list()
# The available languages will be determined based on available
# translations in the //translations/ folder. The exception is (American) English
# since all the text is originally in English already.
# That said, if rare occasions we may want to never show
# the site in English.
if self.config['SUPPORT_ENGLISH']:
languages.append('en_US')
base_path = pathlib.Path(self.app_root) / 'translations'
sybrenUnsubmitted
Done
Inline Actions

You can just use pathlib.Path(self.app_root) / 'translations' instead of calling joinpath()

sybren: You can just use `pathlib.Path(self.app_root) / 'translations'` instead of calling `joinpath()`
sybrenUnsubmitted
Done
Inline Actions

No need to construct a list to pass it to languages.extend(), use a generator expression instead.

sybren: No need to construct a list to pass it to `languages.extend()`, use a generator expression…
sybrenUnsubmitted
Not Done
Inline Actions

You can drop the superfluous parentheses.

sybren: You can drop the superfluous parentheses.
if not base_path.is_dir():
self.log.debug('Project has no translations folder: %s', base_path)
else:
languages.extend(i.name for i in base_path.iterdir() if i.is_dir())
# Use set for quicker lookup
self.languages = set(languages)
self.log.info('Available languages: %s' % ', '.join(self.languages))
def _config_translations(self):
"""
Initialize translations variable.
The BABEL_TRANSLATION_DIRECTORIES has the folder for the compiled
translations files. It uses ; separation for the extension folders.
"""
self.log.info('Configure translations')
translations_path = pathlib.Path(__file__).parents[1].joinpath('translations')
self.config['BABEL_TRANSLATION_DIRECTORIES'] = str(translations_path)
babel = Babel(self)
self.set_languages(translations_path)
# get_locale() is registered as a callback for locale selection.
sybrenUnsubmitted
Done
Inline Actions

This inner function isn't called here, and it looks like it will just immediately be garbage collected. My guess is that somehow this is prevented by the @babel.localeselector annotation, but this should be made explicit.

sybren: This inner function isn't called here, and it looks like it will just immediately be garbage…
# That prevents the function from being garbage collected.
sybrenUnsubmitted
Done
Inline Actions

Annotate the return type.

sybren: Annotate the return type.
@babel.localeselector
def get_locale() -> str:
"""
Callback runs before each request to give us a chance to choose the
sybrenUnsubmitted
Done
Inline Actions

Why is it necessary to both return the locale *and* set it on g.locale? This should be included in the docstring, as it hints as how the entire translation system integrates with the rest of Pillar.

sybren: Why is it necessary to both return the locale *and* set it on `g.locale`? This should be…
language to use when producing its response.
We set g.locale to be able to access it from the template pages.
We still need to return it explicitly, since this function is
called as part of the babel translation framework.
sybrenUnsubmitted
Not Done
Inline Actions

An if not with an else clause is hard to read. This would be simpler to parse mentally:

if self.config['USE_I18N']:
    g.locale = request.accept_languages.best_match(self.languages, self.default_locale)
else:
    g.locale = 'en_US'
return g.locale

Every access to g requires going through a LocalProxy object. It'll be faster to determine the locale and store it in a local variable, then assign that to g.locale and return the local variable:

if self.config['USE_I18N']:
    locale = request.accept_languages.best_match(self.languages, self.default_locale)
else:
    locale = 'en_US'
g.locale = locale
return locale

Since this happens on every request, performance is important.

sybren: An `if not` with an `else` clause is hard to read. This would be simpler to parse mentally…
We are using the 'Accept-Languages' header to match the available
translations with the user supported languages.
"""
locale = request.accept_languages.best_match(
self.languages, self.default_locale)
g.locale = locale
return locale
def load_extension(self, pillar_extension, url_prefix): def load_extension(self, pillar_extension, url_prefix):
from .extension import PillarExtension from .extension import PillarExtension
if not isinstance(pillar_extension, PillarExtension): if not isinstance(pillar_extension, PillarExtension):
if self.config.get('DEBUG'): if self.config.get('DEBUG'):
for cls in type(pillar_extension).mro(): for cls in type(pillar_extension).mro():
self.log.error('class %42r (%i) is %42r (%i): %s', self.log.error('class %42r (%i) is %42r (%i): %s',
cls, id(cls), PillarExtension, id(PillarExtension), cls, id(cls), PillarExtension, id(PillarExtension),
▲ Show 20 Lines • Show All 43 Lines • ▼ Show 20 Lines def load_extension(self, pillar_extension, url_prefix):
(pillar_extension.name, pillar_ext_prefix) (pillar_extension.name, pillar_ext_prefix)
url = key.replace(pillar_ext_prefix, pillar_url_prefix) url = key.replace(pillar_ext_prefix, pillar_url_prefix)
collection.setdefault('datasource', {}).setdefault('source', key) collection.setdefault('datasource', {}).setdefault('source', key)
collection.setdefault('url', url) collection.setdefault('url', url)
self.config['DOMAIN'].update(eve_settings['DOMAIN']) self.config['DOMAIN'].update(eve_settings['DOMAIN'])
# Configure the extension translations
trpath = pillar_extension.translations_path
sybrenUnsubmitted
Done
Inline Actions

I18 is an incomplete name, it should be I18N, which is short for "internationalization".
Also flip the condition and return early, so

if not self.config['USE_I18N'}:
    return

That'll allow you to un-indent the remainder of this function.

sybren: `I18` is an incomplete name, it should be `I18N`, which is short for "internationalization".
if not trpath:
sybrenUnsubmitted
Not Done
Inline Actions

Add a debug log entry that explains it's going to skip I18N stuff.

sybren: Add a debug log entry that explains it's going to skip I18N stuff.
self.log.debug('Extension %s does not have a translations folder',
sybrenUnsubmitted
Done
Inline Actions

Same as above, can be flipped to if not trpath: return, un-indenting the remainder of the function by another level.

sybren: Same as above, can be flipped to `if not trpath: return`, un-indenting the remainder of the…
sybrenUnsubmitted
Not Done
Inline Actions

Don't use f-strings in logging. Now the string will be formatted even before doing the self.log.debug() call, which might drop the formatted string whenever debug-level logging is disabled. Use percent formatting and pass format arguments as extra args to self.log.debug().

sybren: Don't use f-strings in logging. Now the string will be formatted even before doing the `self.
pillar_extension.name)
return
sybrenUnsubmitted
Not Done
Inline Actions

Add a debug log entry that explains this extension doesn't have a translations path

sybren: Add a debug log entry that explains this extension doesn't have a translations path
self.log.info('Extension %s: adding translations path %s',
sybrenUnsubmitted
Done
Inline Actions

The message isn't reflecting the code. trpath can exist as a file, symlink, or socket, and the message will tell you it doesn't exist at all.

sybren: The message isn't reflecting the code. `trpath` can exist as a file, symlink, or socket, and…
sybrenUnsubmitted
Done
Inline Actions

Use f-strings for formatting, so f'Translation path {trpath} for extension {pillar_extension.name} does not exist'
Don't end exception messages in a period.

sybren: Use f-strings for formatting, so `f'Translation path {trpath} for extension {pillar_extension.
pillar_extension.name, trpath)
# Babel requires semi-colon string separation
sybrenUnsubmitted
Not Done
Inline Actions

This can never happen, given how the translations_path property is written.

sybren: This can never happen, given how the `translations_path` property is written.
self.config['BABEL_TRANSLATION_DIRECTORIES'] += ';' + str(trpath)
def _config_jinja_env(self): def _config_jinja_env(self):
# Start with the extensions... # Start with the extensions...
paths_list = [ paths_list = [
jinja2.FileSystemLoader(path) jinja2.FileSystemLoader(path)
for path in reversed(self.pillar_extensions_template_paths) for path in reversed(self.pillar_extensions_template_paths)
] ]
# ...then load Pillar paths. # ...then load Pillar paths.
▲ Show 20 Lines • Show All 203 Lines • ▼ Show 20 Lines def handle_sdk_precondition_failed(self, error):
error.code = 412 error.code = 412
return self.pillar_error_handler(error) return self.pillar_error_handler(error)
def handle_sdk_resource_invalid(self, error): def handle_sdk_resource_invalid(self, error):
self.log.info('Forwarding ResourceInvalid exception to client: %s', error, exc_info=True) self.log.info('Forwarding ResourceInvalid exception to client: %s', error, exc_info=True)
# Raising a Werkzeug 422 exception doens't work, as Flask turns it into a 500. # Raising a Werkzeug 422 exception doens't work, as Flask turns it into a 500.
return 'The submitted data could not be validated.', 422 return _('The submitted data could not be validated.'), 422
def handle_sdk_method_not_allowed(self, error): def handle_sdk_method_not_allowed(self, error):
"""Forwards 405 Method Not Allowed to the client. """Forwards 405 Method Not Allowed to the client.
This is actually not fair, as a 405 between Pillar and Pillar-Web This is actually not fair, as a 405 between Pillar and Pillar-Web
doesn't imply that the request the client did on Pillar-Web is not doesn't imply that the request the client did on Pillar-Web is not
allowed. However, it does allow us to debug this if it happens, by allowed. However, it does allow us to debug this if it happens, by
watching for 405s in the browser. watching for 405s in the browser.
▲ Show 20 Lines • Show All 222 Lines • Show Last 20 Lines