Skip to content

Commit

Permalink
[#1515] Remove worker container from local environments (#1552)
Browse files Browse the repository at this point in the history
* remove confusing WORKER_ENABLED config var

* default task_always_eager to True

* override celery task_always_eager in tests

* remove unused import

* removes the worker container as a dependency for webserver

* only reload files in src/

* add redis to fides deps

* use loguru for all logs in the setup_server hook to prevent regular logger logs being swallowed

* remove unused import, add type ignore

* update changelog
  • Loading branch information
seanpreston authored Oct 27, 2022
1 parent 6254c26 commit 3a5155e
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 48 deletions.
1 change: 1 addition & 0 deletions .fides/celery.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
event_queue_prefix = "fidesops_worker"
task_default_queue = "fidesops"
task_always_eager = true
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,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/)

### Fixed

Expand Down
18 changes: 18 additions & 0 deletions docker-compose.worker.yml
Original file line number Diff line number Diff line change
@@ -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
23 changes: 2 additions & 21 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -12,7 +12,7 @@ services:
depends_on:
fides-db:
condition: service_healthy
worker:
redis:
condition: service_started
expose:
- 8080
Expand All @@ -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"
Expand Down Expand Up @@ -114,24 +113,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

Expand Down
2 changes: 0 additions & 2 deletions docs/fides/docs/installation/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
27 changes: 10 additions & 17 deletions src/fides/api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -184,18 +183,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."
)
Expand All @@ -206,26 +203,26 @@ 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()
update_saas_configs(registry, db)
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()

logging.debug("Sending startup analytics events...")
log.debug("Sending startup analytics events...")
await send_analytics_event(
AnalyticsEvent(
docker=in_docker_container(),
Expand All @@ -234,10 +231,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,
Expand Down
10 changes: 3 additions & 7 deletions src/fides/ctl/core/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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."""
Expand Down Expand Up @@ -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}", # type: ignore
)


Expand Down
1 change: 0 additions & 1 deletion src/fides/ctl/core/config/execution_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions tests/ops/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down

0 comments on commit 3a5155e

Please sign in to comment.