Skip to content

Commit

Permalink
Removes il8n from log messages and adds docs
Browse files Browse the repository at this point in the history
il8n should only be for using facing strings. This updates the code as
best as I could tell the difference, and also documents that expectation
for plugin writers.

closes #2477
  • Loading branch information
bmbouter authored and mdellweg committed Apr 29, 2022
1 parent 3310845 commit f292fb8
Show file tree
Hide file tree
Showing 26 changed files with 116 additions and 111 deletions.
2 changes: 2 additions & 0 deletions CHANGES/2477.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Removed il8n from the logs written so they will always show up in English for speedy resolution of
error messages. All user facing strings are still expected to be il8n.
2 changes: 2 additions & 0 deletions CHANGES/plugin_api/2477.doc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Added docs on the expectation that all user-facing strings are i8ln wrapped with gettext, but log
messages are not.
16 changes: 16 additions & 0 deletions docs/plugins/plugin-writer/concepts/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -516,3 +516,19 @@ is allowed to handle. This includes two types of "checksum handling":
requested that is not listed in the ``ALLOWED_CONTENT_CHECKSUMS`` setting. This is a convenience
facility allowing plugin writers to not check the ``ALLOWED_CONTENT_CHECKSUMS`` setting
themselves.


.. _il8n-expectations:

Internationalization Expectations
---------------------------------

pulpcore and its plugins are expected to internationalize all user-facing strings using Python's
gettext facilities. This allows Pulp to be translated to other languages and be more usable for a
broader base of users.

Administrator facing strings are expected *not* to be internationalized. These include all log
statements, migration output print statements, django management commands, etc. These not being
internationalized will remain in English. This expectation was formed after feedback from
multi-language speakers who believe having error messages for admins in English would reduce the
time to finding a fix and was generally less surprising.
11 changes: 5 additions & 6 deletions pulpcore/app/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,9 @@ def __init__(self, app_name, app_module):
try:
self.version
except AttributeError:
msg = _(
"The plugin `{}` is missing a version declaration. Starting with "
"pulpcore==3.10, plugins are required to define their version on the "
"PulpPluginAppConfig subclass."
msg = (
"The plugin `{}` is missing a version declaration. Starting with pulpcore==3.10, "
"plugins are required to define their version on the PulpPluginAppConfig subclass."
)
raise ImproperlyConfigured(msg.format(self.label))

Expand Down Expand Up @@ -218,7 +217,7 @@ def _populate_access_policies(sender, apps, verbosity, **kwargs):
if created:
if verbosity >= 1:
print(
_("Access policy for {viewset_name} created.").format(
"Access policy for {viewset_name} created.".format(
viewset_name=viewset_name
)
)
Expand All @@ -228,7 +227,7 @@ def _populate_access_policies(sender, apps, verbosity, **kwargs):
db_access_policy.save()
if verbosity >= 1:
print(
_("Access policy for {viewset_name} updated.").format(
"Access policy for {viewset_name} updated.".format(
viewset_name=viewset_name
)
)
Expand Down
4 changes: 2 additions & 2 deletions pulpcore/app/management/commands/add-signing-service.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Command(BaseCommand):
This command is in tech-preview.
"""

help = _("Adds a new AsciiArmoredDetachedSigningService. [tech-preview]")
help = "Adds a new AsciiArmoredDetachedSigningService. [tech-preview]"

def add_arguments(self, parser):
parser.add_argument(
Expand Down Expand Up @@ -75,7 +75,7 @@ def handle(self, *args, **options):
raise CommandError(str(e))

print(
_("Successfully added signing service {name} for key {fingerprint}.").format(
("Successfully added signing service {name} for key {fingerprint}.").format(
name=name, fingerprint=fingerprint
)
)
2 changes: 1 addition & 1 deletion pulpcore/app/management/commands/analyze-publication.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
class Command(BaseCommand):
"""Django management command for viewing files in a publication and the artifacts on disk."""

help = _(__doc__)
help = __doc__

def add_arguments(self, parser):
"""Set up arguments."""
Expand Down
2 changes: 1 addition & 1 deletion pulpcore/app/management/commands/datarepair-2327.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Command(BaseCommand):
Django management command for repairing incorrectly migrated remote data.
"""

help = _(
help = (
"Repairs issue #2327. A small number of configuration settings may have been "
"corrupted during an upgrade from a previous version of Pulp to a Pulp version "
"between 3.15-3.18, resulting in trouble when syncing or viewing certain remotes. "
Expand Down
34 changes: 14 additions & 20 deletions pulpcore/app/management/commands/handle-artifact-checksums.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class Command(BaseCommand):
Django management command for populating or removing checksums on artifacts.
"""

help = _("Handle missing and forbidden checksums on the artifacts")
help = "Handle missing and forbidden checksums on the artifacts"

def add_arguments(self, parser):
parser.add_argument("--report", action="store_true")
Expand All @@ -44,13 +44,11 @@ def _print_out_repository_version_hrefs(self, repo_versions):
"""
for repo_version in repo_versions.iterator():
self.stdout.write(
_(
"/repositories/{plugin}/{type}/{pk}/versions/{number}/".format(
plugin=repo_version.repository.pulp_type.split(".")[0],
type=repo_version.repository.pulp_type.split(".")[1],
pk=str(repo_version.repository.pk),
number=repo_version.number,
)
"/repositories/{plugin}/{type}/{pk}/versions/{number}/".format(
plugin=repo_version.repository.pulp_type.split(".")[0],
type=repo_version.repository.pulp_type.split(".")[1],
pk=str(repo_version.repository.pk),
number=repo_version.number,
)
)

Expand All @@ -69,11 +67,11 @@ def _show_on_demand_content(self, checksums):
repo_versions = RepositoryVersion.objects.with_content(content).select_related("repository")

self.stdout.write(
_("Found {} on-demand content units with forbidden checksums.").format(content.count())
"Found {} on-demand content units with forbidden checksums.".format(content.count())
)
if content.count():
self.stdout.write(
_("There is approx {:.2f}Mb of content to be downloaded.").format(
"There is approx {:.2f}Mb of content to be downloaded.".format(
ras_size / (1024**2)
)
)
Expand All @@ -100,13 +98,13 @@ def _show_immediate_content(self, forbidden_checksums):
repo_versions = RepositoryVersion.objects.with_content(content).select_related("repository")

self.stdout.write(
_("Found {} downloaded content units with forbidden or missing checksums.").format(
"Found {} downloaded content units with forbidden or missing checksums.".format(
content.count()
)
)
if content.count() > 0:
self.stdout.write(
_("There is approx. {:.2f}Mb content data to be re-hashed.").format(
"There is approx. {:.2f}Mb content data to be re-hashed.".format(
artifacts.aggregate(Sum("size"))["size__sum"] / (1024**2)
)
)
Expand Down Expand Up @@ -150,10 +148,8 @@ def _report(self, allowed_checksums):
)

self.stderr.write(
_(
"Warning: the handle-artifact-checksums report is in "
"tech preview and may change in the future."
)
"Warning: the handle-artifact-checksums report is in tech preview and may change in "
"the future."
)
self._show_on_demand_content(forbidden_checksums)
self._show_immediate_content(forbidden_checksums)
Expand Down Expand Up @@ -197,7 +193,7 @@ def handle(self, *args, **options):

if hrefs:
raise CommandError(
_("Some files that were missing could not be restored: {}").format(hrefs)
"Some files that were missing could not be restored: {}".format(hrefs)
)

forbidden_checksums = set(constants.ALL_KNOWN_CONTENT_CHECKSUMS).difference(
Expand All @@ -208,9 +204,7 @@ def handle(self, *args, **options):
update_params = {f"{checksum}": None}
artifacts_qs = Artifact.objects.filter(**search_params)
if artifacts_qs.exists():
self.stdout.write(
_("Removing forbidden checksum {} from database").format(checksum)
)
self.stdout.write("Removing forbidden checksum {} from database".format(checksum))
artifacts_qs.update(**update_params)

self.stdout.write(_("Finished aligning checksums with settings.ALLOWED_CONTENT_CHECKSUMS"))
19 changes: 7 additions & 12 deletions pulpcore/app/management/commands/remove-plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Command(BaseCommand):
This command is in tech-preview.
"""

help = _(
help = (
"[tech-preview] Disable a Pulp plugin and remove all the relevant data from the database. "
"Destructive!"
)
Expand All @@ -55,9 +55,7 @@ def _check_pulp_services(self):
waiting_time = max(settings.CONTENT_APP_TTL, settings.WORKER_TTL)
check_started = time.time()
self.stdout.write(
_("Checking if Pulp services are running, it can take up to {}s...").format(
waiting_time
)
"Checking if Pulp services are running, it can take up to {}s...".format(waiting_time)
)
while is_pulp_running and (time.time() - check_started) < waiting_time:
is_pulp_running = (
Expand All @@ -68,11 +66,8 @@ def _check_pulp_services(self):

if is_pulp_running:
raise CommandError(
_(
"The command can't be used when Pulp services are running. "
"Please stop the services: pulpcore-api, pulpcore-content and all "
"pulpcore-worker@*."
)
"The command can't be used when Pulp services are running. Please stop the "
"services: pulpcore-api, pulpcore-content and all pulpcore-worker@*."
)

def _remove_indirect_plugin_data(self, app_label):
Expand Down Expand Up @@ -127,7 +122,7 @@ def _remove_plugin_data(self, app_label):
if models_to_delete:
# Never-happen case
raise CommandError(
_(
(
"Data for the following models can't be removed: {}. Please contact plugin "
"maintainers."
).format(list(models_to_delete))
Expand Down Expand Up @@ -178,7 +173,7 @@ def handle(self, *args, **options):
available_plugins = {app.label for app in pulp_plugin_configs()} - {"core"}
if plugin_name not in available_plugins:
raise CommandError(
_(
(
"Plugin name is incorrectly specified or plugin is not installed. Please "
"specify one of the following plugin names: {}."
).format(list(available_plugins))
Expand All @@ -193,7 +188,7 @@ def handle(self, *args, **options):
self._unapply_migrations(app_label=plugin_name)

self.stdout.write(
_(
(
"Successfully removed the {} plugin data. It is ready to be uninstalled. "
"NOTE: Please do uninstall, otherwise `pulp status` might not show you the correct "
"list of plugins available."
Expand Down
2 changes: 1 addition & 1 deletion pulpcore/app/management/commands/reset-admin-password.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Command(BaseCommand):
Django management command for resetting the password of the 'admin' user in Pulp.
"""

help = _('Resets "admin" user\'s password.')
help = 'Resets "admin" user\'s password.'

def add_arguments(self, parser):
exclusive = parser.add_mutually_exclusive_group()
Expand Down
4 changes: 2 additions & 2 deletions pulpcore/app/management/commands/stage-profile-summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class Command(BaseCommand):
Django management command for printing a summary report of a Stages API pipeline run.
"""

help = _(
help = (
"Print a summary of a Stages API pipeline run. This command is provided as a tech "
"preview and may not work properly or change in the future."
)
Expand Down Expand Up @@ -55,7 +55,7 @@ def handle(self, *args, **options):
stages_map[row[0]]["interarrival_time_sum"] += row[2]

for stage in stages:
msg = _("{name}\n\tservice time average: {srv_avg:4f}\n")
msg = "{name}\n\tservice time average: {srv_avg:4f}\n"
if in_queue_count == 0:
# This is the first queue that gets no put() calls, so avoid DivisionByZero errors
waiting_avg = srv_avg = length_avg = interarrival_avg = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ def handle_artifact_checksums(apps):
Artifact.objects.bulk_update(objs=artifacts, fields=[checksum], batch_size=1000)

if paths:
_logger.warn(
_(
_logger.warning(
(
"Missing files needed to update artifact {checksum} checksum: {paths}. "
"Please run 'pulpcore-manager handle-artifact-checksums'."
).format(checksum=checksum, paths=paths)
Expand All @@ -66,8 +66,8 @@ def run_handle_artifact_checksums(apps, schema_editor):
try:
handle_artifact_checksums(apps)
except Exception as exc:
_logger.warn(
_(
_logger.warning(
(
"Failed to update checksums for artifacts: {}. "
"Please run 'pulpcore-manager handle-artifact-checksums'."
).format(exc)
Expand Down
4 changes: 1 addition & 3 deletions pulpcore/app/models/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,7 @@ def cleanup_old_versions(self):
self.retain_repo_versions :
]:
_logger.info(
_("Deleting repository version {} due to version retention limit.").format(
version
)
"Deleting repository version {} due to version retention limit.".format(version)
)
version.delete()

Expand Down
2 changes: 1 addition & 1 deletion pulpcore/app/models/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ def set_completed(self):
.update(state=TASK_STATES.COMPLETED, finished_at=timezone.now())
)
if rows != 1:
msg = _("Task set_completed() occurred but Task %s is already in final state")
msg = "Task set_completed() occurred but Task %s is already in final state"
_logger.warning(msg % self.pk)
self.refresh_from_db()

Expand Down
19 changes: 9 additions & 10 deletions pulpcore/app/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import sys

from contextlib import suppress
from gettext import gettext as _
from importlib import import_module
from logging import getLogger
from pathlib import Path
Expand Down Expand Up @@ -304,7 +303,7 @@
"CONTENT_ORIGIN",
must_exist=True,
messages={
"must_exist_true": _(
"must_exist_true": (
"CONTENT_ORIGIN is a required setting but it was not configured. This may be caused "
"by invalid read permissions of the settings file. Note that CONTENT_ORIGIN is set by "
"the installer automatically."
Expand All @@ -317,7 +316,7 @@
redis_host_validator = Validator("REDIS_HOST", must_exist=True, when=cache_enabled_validator)
redis_port_validator = Validator("REDIS_PORT", must_exist=True, when=cache_enabled_validator)
cache_validator = redis_url_validator | (redis_host_validator & redis_port_validator)
cache_validator.messages["combined"] = _(
cache_validator.messages["combined"] = (
"CACHE_ENABLED is enabled but it requires to have REDIS configured. Please check "
"https://docs.pulpproject.org/pulpcore/configuration/settings.html#redis-settings "
"for more information."
Expand All @@ -336,7 +335,7 @@
"ALLOWED_CONTENT_CHECKSUMS",
condition=lambda x: len(set(x).difference(constants.ALL_KNOWN_CONTENT_CHECKSUMS)) == 0,
messages={
"condition": _(
"condition": (
"ALLOWED_CONTENT_CHECKSUMS may only contain algorithms known to pulp - see "
"constants.ALL_KNOWN_CONTENT_CHECKSUMS for the allowed list."
)
Expand All @@ -347,7 +346,7 @@
"API_ROOT",
condition=lambda x: x.startswith("/") and x.endswith("/"),
messages={
"condition": _("The API_ROOT must start and end with a '/', currently it is '{value}'")
"condition": ("The API_ROOT must start and end with a '/', currently it is '{value}'")
},
)

Expand Down Expand Up @@ -384,7 +383,7 @@
Fernet(key_file.read())
except Exception as ex:
raise ImproperlyConfigured(
_("Could not load DB_ENCRYPTION_KEY file '{file}': {err}").format(
("Could not load DB_ENCRYPTION_KEY file '{file}': {err}").format(
file=DB_ENCRYPTION_KEY, err=ex
)
)
Expand All @@ -405,7 +404,7 @@
row = cursor.fetchone()
if row[0] > 0:
raise ImproperlyConfigured(
_(
(
"There have been identified artifacts missing checksum '{}'. "
"Run 'pulpcore-manager handle-artifact-checksums' first to populate "
"missing artifact checksums."
Expand All @@ -419,7 +418,7 @@
row = cursor.fetchone()
if row[0] > 0:
raise ImproperlyConfigured(
_(
(
"There have been identified artifacts with forbidden checksum '{}'. "
"Run 'pulpcore-manager handle-artifact-checksums' first to unset "
"forbidden checksums."
Expand All @@ -436,8 +435,8 @@
)
row = cursor.fetchone()
if row[0] > 0:
_logger.warn(
_(
_logger.warning(
(
"Warning: detected remote content without allowed checksums. "
"Run 'pulpcore-manager handle-artifact-checksums --report' to "
"view this content."
Expand Down
Loading

0 comments on commit f292fb8

Please sign in to comment.