ambient-innovation / ambient-toolbox

Helpers and tools for Python and web-development with Django
https://pypi.org/project/ambient-toolbox/
MIT License
8 stars 4 forks source link

Hard-fail if improper settings are detected #9

Closed mastacheata closed 1 year ago

mastacheata commented 1 year ago

returning False doesn't break any surrounding processes (the command still exits with code 0)

Raise an Exception to make sure a process calling this will either fail or handle the Exception properly

mastacheata commented 1 year ago

We have this code in a service file that automatically dumps the production database and runs the scrubber:

    def _run_django_scrubber(self):
        print('# Running django custom scrubber command')

        # Scrubber command
        django_scrubber_command = [
            f'DJANGO_DEBUG={self.env.bool("DJANGO_DEBUG")}',
            f'DJANGO_DATABASE_URL="{self.env.str("DJANGO_DATABASE_LOCAL_URL")}"',
            'python',
            'manage.py',
            'custom_scrub',
        ]

        # We don't need a logfile because Sentry is activated
        shell_command = subprocess.Popen(' '.join(django_scrubber_command), shell=True)
        exit_code = shell_command.wait()
        if exit_code != 0:
            raise AnonymisationError(f'Scrubber process exited with error code {exit_code}.')

This code will only cause the process to be interruped if the scrubbing process actually fails. Returning False from a Django-Command does not cause the command to exit with an error status code, though,

Therefore raising an Exception from the AbstractScrubbngService is absolutely necessary and als what an onlooker without in-depth knowledge of the code might expect.