management: Disable Sentry for management commands run interactively.

This adds `--automated` and `--no-automated` flags to all Zulip
management commands, whose default is based on if STDIN is a TTY.
This enables cron jobs and supervisor commands to continue to report
to Sentry, and manually-run commands (when reporting to Sentry does
not provide value, since the user can see them) to not.

Note that this only applies to Zulip commands -- core Django
commands (e.g. `./manage.py`) do not grow support for `--automated`
and will always report exceptions to Sentry.

`manage.py` subcommands in the `upgrade` and `restart-server` paths
are marked as `--automated`, since those may be run semi-unattended,
and they are useful to log to Sentry.
This commit is contained in:
Alex Vandiver 2024-05-24 13:55:37 +00:00 committed by Tim Abbott
parent 88be3246a0
commit 2218de0399
5 changed files with 30 additions and 12 deletions

View File

@ -38,13 +38,15 @@ command to write a new one. Some good examples are
is good, but we have a few pieces advice specific to the Zulip is good, but we have a few pieces advice specific to the Zulip
project. project.
- If you need to access a realm or user, use the `ZulipBaseCommand` - Inherit from the `ZulipBaseCommand` class in
class in `zerver/lib/management.py` so you don't need to write the `zerver/lib/management.py`; this will add some helpful general
tedious code of looking those objects up. This is especially flags, as well as tools for adding and parsing `--realm` and
important for users, since the library handles the issues around `--user` flags, so you don't need to write the tedious code of
looking up users by email well (if there's a unique user with that looking those objects up. This is especially important for users,
email, just modify it without requiring the user to specify the since the library handles the issues around looking up users by
realm as well, but if there's a collision, throw a nice error). email well (if there's a unique user with that email, just modify it
without requiring the user to specify the realm as well, but if
there's a collision, throw a nice error).
- Avoid writing a lot of code in management commands; management - Avoid writing a lot of code in management commands; management
commands are annoying to unit test, and thus easier to maintain if commands are annoying to unit test, and thus easier to maintain if
all the interesting logic is in a nice function that is unit tested all the interesting logic is in a nice function that is unit tested

View File

@ -2,4 +2,4 @@
set -e set -e
/home/zulip/deployments/current/manage.py send_zulip_update_announcements /home/zulip/deployments/current/manage.py send_zulip_update_announcements --automated

View File

@ -158,7 +158,8 @@ def fill_memcached_caches() -> None:
if HAS_FILLED_CACHES or migrations_needed: if HAS_FILLED_CACHES or migrations_needed:
return return
subprocess.check_call( subprocess.check_call(
["./manage.py", "fill_memcached_caches", "--skip-checks"], preexec_fn=su_to_zulip ["./manage.py", "fill_memcached_caches", "--automated", "--skip-checks"],
preexec_fn=su_to_zulip,
) )
HAS_FILLED_CACHES = True HAS_FILLED_CACHES = True
@ -486,7 +487,7 @@ if args.audit_fts_indexes:
logging.info("Correcting full-text search indexes for updated dictionary files") logging.info("Correcting full-text search indexes for updated dictionary files")
logging.info("This may take a while but the server should work while it runs.") logging.info("This may take a while but the server should work while it runs.")
subprocess.check_call( subprocess.check_call(
["./manage.py", "audit_fts_indexes", "--skip-checks"], preexec_fn=su_to_zulip ["./manage.py", "audit_fts_indexes", "--automated", "--skip-checks"], preexec_fn=su_to_zulip
) )
if not args.skip_purge_old_deployments: if not args.skip_purge_old_deployments:

View File

@ -56,7 +56,7 @@ if not args.skip_checks:
if args.fill_cache: if args.fill_cache:
logging.info("Filling memcached caches") logging.info("Filling memcached caches")
subprocess.check_call(["./manage.py", "fill_memcached_caches", "--skip-checks"]) subprocess.check_call(["./manage.py", "fill_memcached_caches", "--automated", "--skip-checks"])
current_symlink = os.path.join(DEPLOYMENTS_DIR, "current") current_symlink = os.path.join(DEPLOYMENTS_DIR, "current")
last_symlink = os.path.join(DEPLOYMENTS_DIR, "last") last_symlink = os.path.join(DEPLOYMENTS_DIR, "last")

View File

@ -2,7 +2,7 @@
import logging import logging
import os import os
import sys import sys
from argparse import ArgumentParser, RawTextHelpFormatter, _ActionsContainer from argparse import ArgumentParser, BooleanOptionalAction, RawTextHelpFormatter, _ActionsContainer
from dataclasses import dataclass from dataclasses import dataclass
from functools import reduce, wraps from functools import reduce, wraps
from typing import Any, Dict, Optional, Protocol from typing import Any, Dict, Optional, Protocol
@ -78,9 +78,24 @@ class ZulipBaseCommand(BaseCommand):
@override @override
def create_parser(self, prog_name: str, subcommand: str, **kwargs: Any) -> CommandParser: def create_parser(self, prog_name: str, subcommand: str, **kwargs: Any) -> CommandParser:
parser = super().create_parser(prog_name, subcommand, **kwargs) parser = super().create_parser(prog_name, subcommand, **kwargs)
parser.add_argument(
"--automated",
help="This command is run non-interactively (enables Sentry, etc)",
action=BooleanOptionalAction,
default=not sys.stdin.isatty(),
)
parser.formatter_class = RawTextHelpFormatter parser.formatter_class = RawTextHelpFormatter
return parser return parser
@override
def execute(self, *args: Any, **options: Any) -> None:
if settings.SENTRY_DSN and not options["automated"]: # nocoverage
import sentry_sdk
# This deactivates Sentry
sentry_sdk.init()
super().execute(*args, **options)
def add_realm_args( def add_realm_args(
self, parser: ArgumentParser, *, required: bool = False, help: Optional[str] = None self, parser: ArgumentParser, *, required: bool = False, help: Optional[str] = None
) -> None: ) -> None: