From 7298fb5f0951003dd0978d190fbeac25501959ec Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Mon, 24 Oct 2022 17:39:22 -0400 Subject: [PATCH 01/10] remove confusing WORKER_ENABLED config var --- docker-compose.yml | 1 - docs/fides/docs/installation/configuration.md | 2 -- src/fides/api/main.py | 4 ---- src/fides/ctl/core/config/execution_settings.py | 1 - 4 files changed, 8 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 033c02704d..6d2c213d6a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -25,7 +25,6 @@ services: FIDES__CLI__SERVER_PORT: "8080" FIDES__DATABASE__SERVER: "fides-db" FIDES__DEV_MODE: "True" - FIDES__EXECUTION__WORKER_ENABLED: "True" FIDES__REDIS__ENABLED: "True" FIDES__TEST_MODE: "True" FIDES__USER__ANALYTICS_OPT_OUT: "True" diff --git a/docs/fides/docs/installation/configuration.md b/docs/fides/docs/installation/configuration.md index 637b7deec0..dc14f9ce03 100644 --- a/docs/fides/docs/installation/configuration.md +++ b/docs/fides/docs/installation/configuration.md @@ -63,7 +63,6 @@ task_retry_backoff = 1 subject_identity_verification_required = false task_retry_count = 0 task_retry_delay = 1 -worker_enabled = false [admin_ui] enabled = true @@ -142,7 +141,6 @@ The `fides.toml` file should specify the following variables: |`require_manual_request_approval` | bool | `False` | Whether privacy requests require explicit approval to execute. | |`masking_strict` | bool | `True` | If set to `True`, only use UPDATE requests to mask data. If `False`, Fides will use any defined DELETE or GDPR DELETE endpoints to remove PII, which may extend beyond the specific data categories that configured in your execution policy. | |`celery_config_path` | string | N/A | An optional override for the [Celery](#celery-configuration) configuration file path. | -|`worker_enabled` | bool | `True` | By default, Fides uses a dedicated [Celery worker](#celery-configuration) to process privacy requests asynchronously. Setting `worker_enabled` to `False` will run the worker on the same node as the webserver. | #### User diff --git a/src/fides/api/main.py b/src/fides/api/main.py index eaeaa69eb0..d5dff57b3d 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -243,10 +243,6 @@ async def setup_server() -> None: ) ) - if not CONFIG.execution.worker_enabled: - logger.info("Starting worker...") - subprocess.Popen(["fides", "worker"]) # pylint: disable=consider-using-with - setup_logging( CONFIG.logging.level, serialize=CONFIG.logging.serialization, diff --git a/src/fides/ctl/core/config/execution_settings.py b/src/fides/ctl/core/config/execution_settings.py index 73e54dafc1..487192ca20 100644 --- a/src/fides/ctl/core/config/execution_settings.py +++ b/src/fides/ctl/core/config/execution_settings.py @@ -13,7 +13,6 @@ class ExecutionSettings(FidesSettings): subject_identity_verification_required: bool = False require_manual_request_approval: bool = False masking_strict: bool = True - worker_enabled: bool = False celery_config_path: str = ".fides/celery.toml" class Config: From 92d859fa14c0718864ba137f7b91f994afba5c12 Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Mon, 24 Oct 2022 17:39:52 -0400 Subject: [PATCH 02/10] default task_always_eager to True --- .fides/celery.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/.fides/celery.toml b/.fides/celery.toml index 71e19044f7..a8f1df9ed3 100644 --- a/.fides/celery.toml +++ b/.fides/celery.toml @@ -1,2 +1,3 @@ event_queue_prefix = "fidesops_worker" task_default_queue = "fidesops" +task_always_eager = true \ No newline at end of file From 60dcaf04467a88812a0ea89ea6e3c529bdb7dd62 Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Mon, 24 Oct 2022 17:53:23 -0400 Subject: [PATCH 03/10] override celery task_always_eager in tests --- tests/ops/conftest.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/ops/conftest.py b/tests/ops/conftest.py index c441a9feca..4b42b7fc17 100644 --- a/tests/ops/conftest.py +++ b/tests/ops/conftest.py @@ -204,6 +204,11 @@ def integration_config(): yield load_toml(["tests/ops/integration_test_config.toml"]) +@pytest.fixture(scope="session") +def celery_config(): + return {"task_always_eager": False} + + @pytest.fixture(autouse=True, scope="session") def celery_enable_logging(): """Turns on celery output logs.""" From 1344eb9e22dcdf612a510130d19462b26bcbdf88 Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Mon, 24 Oct 2022 17:54:33 -0400 Subject: [PATCH 04/10] remove unused import --- src/fides/api/main.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/fides/api/main.py b/src/fides/api/main.py index d5dff57b3d..00eba3db81 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -2,7 +2,6 @@ Contains the code that sets up the API. """ import logging -import subprocess from datetime import datetime, timezone from logging import WARNING from os import getenv From ad9c48461eabc63659671210b0ca81ade460044f Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Mon, 24 Oct 2022 18:03:02 -0400 Subject: [PATCH 05/10] removes the worker container as a dependency for webserver --- docker-compose.worker.yml | 18 ++++++++++++++++++ docker-compose.yml | 20 -------------------- 2 files changed, 18 insertions(+), 20 deletions(-) create mode 100644 docker-compose.worker.yml diff --git a/docker-compose.worker.yml b/docker-compose.worker.yml new file mode 100644 index 0000000000..7fab2fd883 --- /dev/null +++ b/docker-compose.worker.yml @@ -0,0 +1,18 @@ +services: + worker: + image: ethyca/fides:local + command: fides worker + depends_on: + redis: + condition: service_started + restart: always + environment: + FIDES__CONFIG_PATH: ${FIDES__CONFIG_PATH:-/fides/.fides/fides.toml} + FIDES__TEST_MODE: "True" + FIDES__USER__ANALYTICS_OPT_OUT: "True" + volumes: + - type: bind + source: ./ + target: /fides + read_only: False + - /fides/src/fides.egg-info diff --git a/docker-compose.yml b/docker-compose.yml index 6d2c213d6a..50d25eeb61 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -12,8 +12,6 @@ services: depends_on: fides-db: condition: service_healthy - worker: - condition: service_started expose: - 8080 env_file: @@ -113,24 +111,6 @@ services: ports: - "0.0.0.0:6379:6379" - worker: - image: ethyca/fides:local - command: fides worker - depends_on: - redis: - condition: service_started - restart: always - environment: - FIDES__CONFIG_PATH: ${FIDES__CONFIG_PATH:-/fides/.fides/fides.toml} - FIDES__TEST_MODE: "True" - FIDES__USER__ANALYTICS_OPT_OUT: "True" - volumes: - - type: bind - source: ./ - target: /fides - read_only: False - - /fides/src/fides.egg-info - volumes: postgres: null From ea40e87d55b48ff12f3539cf943774046d176c9a Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Mon, 24 Oct 2022 20:42:25 -0400 Subject: [PATCH 06/10] only reload files in src/ --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 50d25eeb61..4bff339003 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ services: fides: image: ethyca/fides:local - command: uvicorn --host 0.0.0.0 --port 8080 --reload fides.api.main:app + command: uvicorn --host 0.0.0.0 --port 8080 --reload --reload-dir src fides.api.main:app healthcheck: test: [ "CMD", "curl", "-f", "http://0.0.0.0:8080/health" ] interval: 20s From f1573d9d33797a5b060830ca7236aac77b1de13f Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Tue, 25 Oct 2022 21:50:22 -0400 Subject: [PATCH 07/10] add redis to fides deps --- docker-compose.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index 4bff339003..81cd74ca60 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -12,6 +12,8 @@ services: depends_on: fides-db: condition: service_healthy + redis: + condition: service_started expose: - 8080 env_file: From 66e684e282020031f588d637e92f8c05d95f431e Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Tue, 25 Oct 2022 22:04:30 -0400 Subject: [PATCH 08/10] use loguru for all logs in the setup_server hook to prevent regular logger logs being swallowed --- src/fides/api/main.py | 25 ++++++++++--------------- src/fides/ctl/core/config/__init__.py | 10 +++------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/fides/api/main.py b/src/fides/api/main.py index 00eba3db81..2819500dcf 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -189,18 +189,16 @@ def configure_routes() -> None: @app.on_event("startup") async def setup_server() -> None: "Run all of the required setup steps for the webserver." - logger.warning( - "Startup configuration: reloading = %s, dev_mode = %s", - CONFIG.hot_reloading, - CONFIG.dev_mode, + log.warning( + f"Startup configuration: reloading = {CONFIG.hot_reloading}, dev_mode = {CONFIG.dev_mode}", ) - logger.warning( - "Startup configuration: pii logging = %s", - getenv("FIDES__LOG_PII", "").lower() == "true", + log_pii = getenv("FIDES__LOG_PII", "").lower() == "true" + log.warning( + f"Startup configuration: pii logging = {log_pii}", ) if logger.getEffectiveLevel() == logging.DEBUG: - logger.warning( + log.warning( "WARNING: log level is DEBUG, so sensitive or personal data may be logged. " "Set FIDES__LOGGING__LEVEL to INFO or higher in production." ) @@ -211,7 +209,7 @@ async def setup_server() -> None: await configure_db(CONFIG.database.sync_database_uri) - logger.info("Validating SaaS connector templates...") + log.info("Validating SaaS connector templates...") registry = load_registry(registry_file) try: db = get_api_session() @@ -219,21 +217,18 @@ async def setup_server() -> None: finally: db.close() - logger.info("Running Redis connection test...") + log.info("Running Redis connection test...") try: get_cache() except (RedisConnectionError, ResponseError) as e: - logger.error("Connection to cache failed: %s", Pii(str(e))) + log.error("Connection to cache failed: %s", Pii(str(e))) return if not scheduler.running: scheduler.start() - logger.info("Starting scheduled request intake...") - initiate_scheduled_request_intake() - - logging.debug("Sending startup analytics events...") + log.debug("Sending startup analytics events...") await send_analytics_event( AnalyticsEvent( docker=in_docker_container(), diff --git a/src/fides/ctl/core/config/__init__.py b/src/fides/ctl/core/config/__init__.py index f9ecd3cc31..688c93df9b 100644 --- a/src/fides/ctl/core/config/__init__.py +++ b/src/fides/ctl/core/config/__init__.py @@ -10,6 +10,7 @@ import toml from fideslib.core.config import load_toml +from loguru import logger as log from pydantic import BaseModel from fides.ctl.core.utils import echo_red @@ -26,8 +27,6 @@ from .user_settings import UserSettings from .utils import DEFAULT_CONFIG_PATH, get_test_mode -logger = logging.getLogger(__name__) - class FidesConfig(BaseModel): """Umbrella class that encapsulates all of the config subsections.""" @@ -65,11 +64,8 @@ def log_all_config_values(self) -> None: self.admin_ui, ]: for key, value in settings.dict().items(): # type: ignore - logger.debug( - "Using config: %s%s = %s", - settings.Config.env_prefix, # type: ignore - key.upper(), - value, + log.debug( + f"Using config: {settings.Config.env_prefix}{key.upper()} = {value}", ) From 0d64518cc2a4b3b76700814c96e72f943cee833b Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Tue, 25 Oct 2022 22:13:53 -0400 Subject: [PATCH 09/10] remove unused import, add type ignore --- src/fides/api/main.py | 1 - src/fides/ctl/core/config/__init__.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/fides/api/main.py b/src/fides/api/main.py index 2819500dcf..84e89b1b17 100644 --- a/src/fides/api/main.py +++ b/src/fides/api/main.py @@ -61,7 +61,6 @@ update_saas_configs, ) from fides.api.ops.tasks.scheduled.scheduler import scheduler -from fides.api.ops.tasks.scheduled.tasks import initiate_scheduled_request_intake from fides.api.ops.util.cache import get_cache from fides.api.ops.util.logger import Pii, get_fides_log_record_factory from fides.api.ops.util.oauth_util import verify_oauth_client diff --git a/src/fides/ctl/core/config/__init__.py b/src/fides/ctl/core/config/__init__.py index 688c93df9b..99e127e2e5 100644 --- a/src/fides/ctl/core/config/__init__.py +++ b/src/fides/ctl/core/config/__init__.py @@ -65,7 +65,7 @@ def log_all_config_values(self) -> None: ]: for key, value in settings.dict().items(): # type: ignore log.debug( - f"Using config: {settings.Config.env_prefix}{key.upper()} = {value}", + f"Using config: {settings.Config.env_prefix}{key.upper()} = {value}", # type: ignore ) From 20281e86d09bbdea2943c8930cac016304b20e46 Mon Sep 17 00:00:00 2001 From: Sean Preston Date: Thu, 27 Oct 2022 11:04:11 -0400 Subject: [PATCH 10/10] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c489fea960..8e0fb22d75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ The types of changes are: * Repository dispatch events are sent to fidesctl-plus and fidesops-plus [#1263](https://github.com/ethyca/fides/pull/1263) * Only the `docs-authors` team members are specified as `CODEOWNERS` [#1446](https://github.com/ethyca/fides/pull/1446) +* Updates the default local configuration to not defer tasks to a worker node [#1552](https://github.com/ethyca/fides/pull/1552/) ### Docs