Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeout troubleshooting #5714

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/backend_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ jobs:
- name: Set Up Python
uses: actions/setup-python@v5
with:
python-version: ${{ env.DEFAULT_PYTHON_VERSION }}
python-version: ${{ matrix.python_version }}
cache: "pip"

- name: Install Nox
Expand Down Expand Up @@ -287,7 +287,7 @@ jobs:
- name: Set Up Python
uses: actions/setup-python@v5
with:
python-version: ${{ env.DEFAULT_PYTHON_VERSION }}
python-version: ${{ matrix.python_version }}
cache: "pip"

- name: Install Nox
Expand Down Expand Up @@ -349,7 +349,7 @@ jobs:
- name: Set Up Python
uses: actions/setup-python@v5
with:
python-version: ${{ env.DEFAULT_PYTHON_VERSION }}
python-version: ${{ matrix.python_version }}
cache: "pip"

- name: Install Nox
Expand Down Expand Up @@ -444,7 +444,7 @@ jobs:
- name: Set Up Python
uses: actions/setup-python@v5
with:
python-version: ${{ env.DEFAULT_PYTHON_VERSION }}
python-version: ${{ matrix.python_version }}
cache: "pip"

- name: Install Nox
Expand Down
9 changes: 6 additions & 3 deletions noxfiles/setup_tests_nox.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,18 @@ def pytest_ctl(session: Session, mark: str, coverage_arg: str) -> None:
session.run(*LOGIN, external=True)
run_command = (
*EXEC,
"timeout",
"--signal=INT",
"360",
# "timeout",
# "--signal=INT",
# "30",
"pytest",
coverage_arg,
"tests/ctl/",
"-m",
mark,
"--full-trace",
"-s",
# "-x",
# "--pdb",
)
session.run(*run_command, external=True)

Expand Down
9 changes: 6 additions & 3 deletions src/fides/api/api/v1/endpoints/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ class DBActions(str, Enum):
],
status_code=status.HTTP_200_OK,
)
async def db_action(action: DBActions, revision: Optional[str] = "head") -> Dict:
def db_action(
action: DBActions,
revision: Optional[str] = "head",
) -> Dict:
"""
Initiate one of the enumerated DBActions.

Expand All @@ -40,7 +43,7 @@ async def db_action(action: DBActions, revision: Optional[str] = "head") -> Dict

if action == DBActions.downgrade:
try:
await migrate_db(database_url=CONFIG.database.sync_database_uri, revision=revision, downgrade=True) # type: ignore[arg-type]
migrate_db(database_url=CONFIG.database.sync_database_uri, revision=revision, downgrade=True) # type: ignore[arg-type]
action_text = "downgrade"
except Exception as e:
logger.exception("Database downgrade failed")
Expand Down Expand Up @@ -68,7 +71,7 @@ async def db_action(action: DBActions, revision: Optional[str] = "head") -> Dict

try:
logger.info("Database being configured...")
await configure_db(CONFIG.database.sync_database_uri, revision=revision)
configure_db(CONFIG.database.sync_database_uri, revision=revision)
except Exception as e:
logger.exception("Database configuration failed: {e}")
raise HTTPException(
Expand Down
17 changes: 12 additions & 5 deletions src/fides/api/app_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,13 @@
from fides.api.api.v1.endpoints.health import HEALTH_ROUTER
from fides.api.api.v1.exception_handlers import ExceptionHandlers
from fides.api.common_exceptions import RedisConnectionError, RedisNotConfigured
from fides.api.db.ctl_session import async_session
from fides.api.db.database import configure_db
from fides.api.db.seed import create_or_update_parent_user
from fides.api.db.seed import (
create_or_update_parent_user,
load_default_resources,
load_samples,
)
from fides.api.models.application_config import ApplicationConfig
from fides.api.oauth.system_manager_oauth_util import (
get_system_fides_key,
Expand Down Expand Up @@ -153,7 +158,7 @@ def log_startup() -> None:
CONFIG.log_all_config_values()


async def run_database_startup(app: FastAPI) -> None:
def run_database_startup(app: FastAPI) -> None:
"""
Perform all relevant database startup activities/configuration for the
application webserver.
Expand All @@ -164,9 +169,11 @@ async def run_database_startup(app: FastAPI) -> None:

if CONFIG.database.automigrate:
try:
await configure_db(
CONFIG.database.sync_database_uri, samples=CONFIG.database.load_samples
)
configure_db(CONFIG.database.sync_database_uri)
# async with async_session() as session:
# await load_default_resources(session)
# if CONFIG.database.load_samples:
# await load_samples(session)
except Exception as e:
logger.error("Error occurred during database configuration: {}", str(e))
else:
Expand Down
19 changes: 5 additions & 14 deletions src/fides/api/db/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
from sqlalchemy_utils.types.encrypted.encrypted_type import InvalidCiphertextError

from fides.api.db.base import Base # type: ignore[attr-defined]
from fides.api.db.ctl_session import async_session
from fides.api.db.seed import load_default_resources, load_samples
from fides.api.util.errors import get_full_exception_name
from fides.core.utils import get_db_engine

Expand Down Expand Up @@ -48,9 +46,8 @@ def downgrade_db(alembic_config: Config, revision: str = "head") -> None:
command.downgrade(alembic_config, revision)


async def migrate_db(
def migrate_db(
database_url: str,
samples: bool = False,
revision: str = "head",
downgrade: bool = False,
) -> None:
Expand All @@ -66,11 +63,6 @@ async def migrate_db(
else:
upgrade_db(alembic_config, revision)

async with async_session() as session:
await load_default_resources(session)
if samples:
await load_samples(session)


def create_db_if_not_exists(database_url: str) -> None:
"""
Expand All @@ -86,9 +78,10 @@ def reset_db(database_url: str) -> None:
"""
log.info("Resetting database...")
engine = get_db_engine(database_url)
engine.dispose()
with engine.connect() as connection:
log.info("Dropping tables...")
Base.metadata.drop_all(connection)
Base.metadata.drop_all(bind=connection)

log.info("Dropping Alembic table...")
migration_context = migration.MigrationContext.configure(connection)
Expand Down Expand Up @@ -121,13 +114,11 @@ def get_db_health(
return ("unhealthy", None)


async def configure_db(
database_url: str, samples: bool = False, revision: Optional[str] = "head"
) -> None:
def configure_db(database_url: str, revision: Optional[str] = "head") -> None:
"""Set up the db to be used by the app."""
try:
create_db_if_not_exists(database_url)
await migrate_db(database_url, samples=samples, revision=revision) # type: ignore[arg-type]
migrate_db(database_url, revision=revision) # type: ignore[arg-type]
except InvalidCiphertextError as cipher_error:
log.error(
"Unable to configure database due to a decryption error! Check to ensure your `app_encryption_key` has not changed."
Expand Down
74 changes: 42 additions & 32 deletions src/fides/api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,51 +69,61 @@ async def lifespan(wrapped_app: FastAPI) -> AsyncGenerator[None, None]:
and must be maintained.
"""
start_time = perf_counter()
logger.info("Starting server setup...")
try:
logger.info("Starting server setup...")

if not CONFIG.dev_mode:
sys.tracebacklimit = 0
if not CONFIG.dev_mode:
sys.tracebacklimit = 0

log_startup()
log_startup()

await run_database_startup(wrapped_app)
logger.debug("Starting database startup...")
run_database_startup(wrapped_app)
logger.debug("Database startup complete")

check_redis()
logger.debug("Checking Redis...")
check_redis()
logger.debug("Redis check complete")

if not scheduler.running:
scheduler.start()
if not async_scheduler.running:
async_scheduler.start()
if not scheduler.running:
logger.debug("Starting scheduler...")
scheduler.start()
if not async_scheduler.running:
logger.debug("Starting async scheduler...")
async_scheduler.start()

# generate and/or cache the identity salt
get_identity_salt()
# generate and/or cache the identity salt
get_identity_salt()

initiate_scheduled_batch_email_send()
initiate_poll_for_exited_privacy_request_tasks()
initiate_scheduled_dsr_data_removal()
initiate_bcrypt_migration_task()
initiate_scheduled_batch_email_send()
initiate_poll_for_exited_privacy_request_tasks()
initiate_scheduled_dsr_data_removal()
initiate_bcrypt_migration_task()

logger.debug("Sending startup analytics events...")
# Avoid circular imports
from fides.api.analytics import in_docker_container, send_analytics_event
logger.debug("Sending startup analytics events...")
# Avoid circular imports
from fides.api.analytics import in_docker_container, send_analytics_event

await send_analytics_event(
AnalyticsEvent(
docker=in_docker_container(),
event=Event.server_start.value,
event_created_at=datetime.now(tz=timezone.utc),
await send_analytics_event(
AnalyticsEvent(
docker=in_docker_container(),
event=Event.server_start.value,
event_created_at=datetime.now(tz=timezone.utc),
)
)
)

# It's just a random bunch of strings when serialized
if not CONFIG.logging.serialization:
logger.info(FIDES_ASCII_ART)
# It's just a random bunch of strings when serialized
if not CONFIG.logging.serialization:
logger.info(FIDES_ASCII_ART)

warn_root_user_enabled()
warn_root_user_enabled()

logger.info("Fides startup complete! v{}", VERSION)
startup_time = round(perf_counter() - start_time, 3)
logger.info("Server setup completed in {} seconds", startup_time)
logger.info("Fides startup complete! v{}", VERSION)
startup_time = round(perf_counter() - start_time, 3)
logger.info("Server setup completed in {} seconds", startup_time)
except Exception as e:
logger.error(f"Startup failed: {str(e)}")
raise
yield # All of this happens before the webserver comes up


Expand Down
2 changes: 0 additions & 2 deletions tests/ctl/api/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from fides.config import FidesConfig


@pytest.mark.skip("Troubleshooting")
def test_db_reset_dev_mode_enabled(
test_config: FidesConfig,
test_client: TestClient,
Expand All @@ -22,7 +21,6 @@ def test_db_reset_dev_mode_enabled(
}


@pytest.mark.skip("Troubleshooting")
def test_db_reset_dev_mode_disabled(
test_config: FidesConfig,
test_config_dev_mode_disabled: FidesConfig, # temporarily switches off config.dev_mode
Expand Down
1 change: 0 additions & 1 deletion tests/ctl/api/test_seed.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def parent_server_config_password_only():

@pytest.mark.unit
class TestFilterDataCategories:
@pytest.mark.skip("this times out on CI")
def test_filter_data_categories_excluded(self) -> None:
"""Test that the filter method works as intended"""
excluded_data_categories = [
Expand Down
13 changes: 3 additions & 10 deletions tests/ctl/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,11 @@ def monkeypatch_requests(test_client, monkeysession) -> None:


@pytest.fixture(scope="session", autouse=True)
@pytest.mark.usefixtures("monkeypatch_requests")
def setup_ctl_db(test_config, test_client, config):
def setup_ctl_db(api_client, config):
"Sets up the database for testing."
assert config.test_mode
assert (
requests.post == test_client.post
) # Sanity check to make sure monkeypatch_requests fixture has run
yield api.db_action(
server_url=test_config.cli.server_url,
headers=config.user.auth_header,
action="reset",
)
assert requests.post != api_client.post
yield api_client.post(url=f"{config.cli.server_url}/v1/admin/db/reset")


@pytest.fixture(scope="session")
Expand Down
Loading