Skip to content

Commit

Permalink
adjust fn used by worker to determine timeseries enabled (#975)
Browse files Browse the repository at this point in the history
  • Loading branch information
adrian-codecov authored Dec 19, 2024
1 parent a27d4e4 commit 6f971be
Show file tree
Hide file tree
Showing 12 changed files with 41 additions and 44 deletions.
4 changes: 2 additions & 2 deletions database/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
from decimal import Decimal

from shared.config import get_config
from shared.timeseries.helpers import is_timeseries_enabled
from shared.utils.ReportEncoder import ReportEncoder
from sqlalchemy import create_engine
from sqlalchemy.orm import Session, scoped_session, sessionmaker

import database.events # noqa: F401
from database.models.timeseries import TimeseriesBaseModel
from helpers.timeseries import timeseries_enabled

from .base import Base

Expand Down Expand Up @@ -44,7 +44,7 @@ def create_session(self):
json_serializer=json_dumps,
)

if timeseries_enabled():
if is_timeseries_enabled():
self.timeseries_engine = create_engine(
self.timeseries_database_url,
json_serializer=json_dumps,
Expand Down
4 changes: 2 additions & 2 deletions database/tests/unit/test_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

class TestDatabaseEngine:
def test_session_get_bind_timeseries_disabled(self, sqlalchemy_connect_url, mocker):
mocker.patch("database.engine.timeseries_enabled", return_value=False)
mocker.patch("database.engine.is_timeseries_enabled", return_value=False)

session_factory = SessionFactory(
database_url=sqlalchemy_connect_url,
Expand All @@ -33,7 +33,7 @@ def test_session_get_bind_timeseries_disabled(self, sqlalchemy_connect_url, mock
assert engine == session_factory.main_engine

def test_session_get_bind_timeseries_enabled(self, sqlalchemy_connect_url, mocker):
mocker.patch("database.engine.timeseries_enabled", return_value=True)
mocker.patch("database.engine.is_timeseries_enabled", return_value=True)

session_factory = SessionFactory(
database_url=sqlalchemy_connect_url,
Expand Down
4 changes: 0 additions & 4 deletions helpers/timeseries.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
from shared.config import get_config


def timeseries_enabled() -> bool:
return get_config("setup", "timeseries", "enabled", default=False)


def backfill_max_batch_size() -> int:
return get_config("setup", "timeseries", "backfill_max_batch_size", default=500)
22 changes: 11 additions & 11 deletions services/tests/test_timeseries.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class TestTimeseriesService(object):
def test_insert_commit_measurement(
self, dbsession, sample_report, repository, mocker
):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(sample_report),
Expand Down Expand Up @@ -140,7 +140,7 @@ def test_insert_commit_measurement(
assert measurement.value == 60.0

def test_save_commit_measurements_no_report(self, dbsession, repository, mocker):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=None,
Expand All @@ -167,7 +167,7 @@ def test_save_commit_measurements_no_report(self, dbsession, repository, mocker)
def test_update_commit_measurement(
self, dbsession, sample_report, repository, mocker
):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(sample_report),
Expand Down Expand Up @@ -218,7 +218,7 @@ def test_update_commit_measurement(
def test_commit_measurement_insert_flags(
self, dbsession, sample_report, repository, mocker
):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(sample_report),
Expand Down Expand Up @@ -291,7 +291,7 @@ def test_commit_measurement_insert_flags(
def test_commit_measurement_update_flags(
self, dbsession, sample_report, repository, mocker
):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(sample_report),
Expand Down Expand Up @@ -390,7 +390,7 @@ def test_commit_measurement_update_flags(
def test_commit_measurement_insert_components(
self, dbsession, sample_report_for_components, repository, mocker
):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(
Expand Down Expand Up @@ -575,7 +575,7 @@ def test_commit_measurement_insert_components(
def test_commit_measurement_update_component(
self, dbsession, sample_report_for_components, repository, mocker
):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(
Expand Down Expand Up @@ -645,7 +645,7 @@ def test_commit_measurement_update_component(
assert measurement.value == 50.0

def test_commit_measurement_no_datasets(self, dbsession, mocker):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)

repository = RepositoryFactory.create()
dbsession.add(repository)
Expand Down Expand Up @@ -786,7 +786,7 @@ def test_backfill_batch_size(self, repository, mocker):
assert batch_size == 100

def test_delete_repository_data(self, dbsession, sample_report, repository, mocker):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(sample_report),
Expand Down Expand Up @@ -825,7 +825,7 @@ def test_delete_repository_data(self, dbsession, sample_report, repository, mock
def test_delete_repository_data_side_effects(
self, dbsession, sample_report, repository, mocker
):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(sample_report),
Expand Down Expand Up @@ -901,7 +901,7 @@ def validate_invariants(repository, other_repository):
== 16
)

mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch(
"services.report.ReportService.get_existing_report_for_commit",
return_value=ReadOnlyReport.create_from_report(
Expand Down
5 changes: 3 additions & 2 deletions services/timeseries.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@

from shared.components import Component
from shared.reports.readonly import ReadOnlyReport
from shared.timeseries.helpers import is_timeseries_enabled
from sqlalchemy.dialects.postgresql import insert
from sqlalchemy.orm import Session

from database.models import Commit, Dataset, Measurement, MeasurementName
from database.models.core import Repository
from database.models.reports import RepositoryFlag
from helpers.timeseries import backfill_max_batch_size, timeseries_enabled
from helpers.timeseries import backfill_max_batch_size
from services.report import ReportService
from services.yaml import get_repo_yaml

Expand All @@ -20,7 +21,7 @@
def save_commit_measurements(
commit: Commit, dataset_names: Iterable[str] = None
) -> None:
if not timeseries_enabled():
if not is_timeseries_enabled():
return

if dataset_names is None:
Expand Down
6 changes: 3 additions & 3 deletions tasks/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.db import transaction as django_transaction
from shared.celery_router import route_tasks_based_on_user_plan
from shared.metrics import Counter, Histogram
from shared.timeseries.helpers import is_timeseries_enabled
from sqlalchemy.exc import (
DataError,
IntegrityError,
Expand All @@ -22,7 +23,6 @@
from helpers.checkpoint_logger.flows import TestResultsFlow, UploadFlow
from helpers.log_context import LogContext, set_log_context
from helpers.telemetry import TimeseriesTimer, log_simple_metric
from helpers.timeseries import timeseries_enabled

log = logging.getLogger("worker")

Expand Down Expand Up @@ -179,7 +179,7 @@ def _commit_django(self):
extra=dict(e=e),
)

if timeseries_enabled():
if is_timeseries_enabled():
try:
django_transaction.commit("timeseries")
except Exception as e:
Expand All @@ -199,7 +199,7 @@ def _rollback_django(self):
extra=dict(e=e),
)

if timeseries_enabled():
if is_timeseries_enabled():
try:
django_transaction.rollback("timeseries")
except Exception as e:
Expand Down
4 changes: 2 additions & 2 deletions tasks/tests/integration/test_timeseries_backfill.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

@pytest.mark.integration
def test_backfill_dataset_run_impl(dbsession, mocker, mock_storage):
mocker.patch("services.timeseries.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=True)
mocker.patch("services.timeseries.is_timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=True)
mocked_app = mocker.patch.object(
TimeseriesBackfillCommitsTask,
"app",
Expand Down
4 changes: 2 additions & 2 deletions tasks/tests/unit/test_timeseries_backfill_commits.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@


def test_backfill_commits_run_impl(dbsession, mocker):
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=True)
mocked_app = mocker.patch.object(
TimeseriesBackfillCommitsTask,
"app",
Expand Down Expand Up @@ -62,7 +62,7 @@ def test_backfill_commits_run_impl(dbsession, mocker):


def test_backfill_commits_run_impl_timeseries_not_enabled(dbsession, mocker):
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=False)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=False)
mock_group = mocker.patch("tasks.timeseries_backfill.group")

task = TimeseriesBackfillCommitsTask()
Expand Down
12 changes: 6 additions & 6 deletions tasks/tests/unit/test_timeseries_backfill_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@


def test_backfill_dataset_run_impl(dbsession, mocker):
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=True)
mock_group = mocker.patch("tasks.timeseries_backfill.group")

repository = RepositoryFactory.create()
Expand Down Expand Up @@ -70,7 +70,7 @@ def test_backfill_dataset_run_impl(dbsession, mocker):


def test_backfill_dataset_run_impl_invalid_dataset(dbsession, mocker):
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=True)
mock_group = mocker.patch("tasks.timeseries_backfill.group")

repository = RepositoryFactory.create()
Expand All @@ -97,7 +97,7 @@ def test_backfill_dataset_run_impl_invalid_dataset(dbsession, mocker):


def test_backfill_dataset_run_impl_invalid_repository(dbsession, mocker):
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=True)
mock_group = mocker.patch("tasks.timeseries_backfill.group")

repository = RepositoryFactory.create()
Expand All @@ -124,7 +124,7 @@ def test_backfill_dataset_run_impl_invalid_repository(dbsession, mocker):


def test_backfill_dataset_run_impl_invalid_start_date(dbsession, mocker):
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=True)
mock_group = mocker.patch("tasks.timeseries_backfill.group")

repository = RepositoryFactory.create()
Expand All @@ -151,7 +151,7 @@ def test_backfill_dataset_run_impl_invalid_start_date(dbsession, mocker):


def test_backfill_dataset_run_impl_invalid_end_date(dbsession, mocker):
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=True)
mock_group = mocker.patch("tasks.timeseries_backfill.group")

repository = RepositoryFactory.create()
Expand All @@ -178,7 +178,7 @@ def test_backfill_dataset_run_impl_invalid_end_date(dbsession, mocker):


def test_backfill_dataset_run_impl_timeseries_not_enabled(dbsession, mocker):
mocker.patch("tasks.timeseries_backfill.timeseries_enabled", return_value=False)
mocker.patch("tasks.timeseries_backfill.is_timeseries_enabled", return_value=False)
mock_group = mocker.patch("tasks.timeseries_backfill.group")

task = TimeseriesBackfillDatasetTask()
Expand Down
10 changes: 5 additions & 5 deletions tasks/tests/unit/test_timeseries_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@


def test_timeseries_delete_run_impl(dbsession, mocker):
mocker.patch("tasks.timeseries_delete.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_delete.is_timeseries_enabled", return_value=True)
delete_repository_data = mocker.patch(
"tasks.timeseries_delete.delete_repository_data"
)
Expand All @@ -24,7 +24,7 @@ def test_timeseries_delete_run_impl(dbsession, mocker):


def test_timeseries_delete_run_impl_invalid_repository(dbsession, mocker):
mocker.patch("tasks.timeseries_delete.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_delete.is_timeseries_enabled", return_value=True)

task = TimeseriesDeleteTask()
res = task.run_impl(
Expand All @@ -35,7 +35,7 @@ def test_timeseries_delete_run_impl_invalid_repository(dbsession, mocker):


def test_timeseries_delete_run_impl_timeseries_not_enabled(dbsession, mocker):
mocker.patch("tasks.timeseries_delete.timeseries_enabled", return_value=False)
mocker.patch("tasks.timeseries_delete.is_timeseries_enabled", return_value=False)

repository = RepositoryFactory.create()
dbsession.add(repository)
Expand All @@ -50,7 +50,7 @@ def test_timeseries_delete_run_impl_timeseries_not_enabled(dbsession, mocker):


def test_timeseries_delete_measurements_only(dbsession, mocker):
mocker.patch("tasks.timeseries_delete.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_delete.is_timeseries_enabled", return_value=True)
delete_repository_measurements = mocker.patch(
"tasks.timeseries_delete.delete_repository_measurements"
)
Expand All @@ -73,7 +73,7 @@ def test_timeseries_delete_measurements_only(dbsession, mocker):


def test_timeseries_delete_measurements_only_unsuccessful(dbsession, mocker):
mocker.patch("tasks.timeseries_delete.timeseries_enabled", return_value=True)
mocker.patch("tasks.timeseries_delete.is_timeseries_enabled", return_value=True)
delete_repository_measurements = mocker.patch(
"tasks.timeseries_delete.delete_repository_measurements"
)
Expand Down
6 changes: 3 additions & 3 deletions tasks/timeseries_backfill.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
timeseries_backfill_dataset_task_name,
timeseries_save_commit_measurements_task_name,
)
from shared.timeseries.helpers import is_timeseries_enabled
from sqlalchemy.orm.session import Session

from app import celery_app
from database.models import Commit, Repository
from database.models.timeseries import Dataset
from helpers.timeseries import timeseries_enabled
from services.timeseries import backfill_batch_size, repository_commits_query
from tasks.base import BaseCodecovTask

Expand All @@ -32,7 +32,7 @@ def run_impl(
dataset_names: Iterable[str],
**kwargs,
):
if not timeseries_enabled():
if not is_timeseries_enabled():
log.warning("Timeseries not enabled")
return {"successful": False}

Expand Down Expand Up @@ -69,7 +69,7 @@ def run_impl(
batch_size: Optional[int] = None,
**kwargs,
):
if not timeseries_enabled():
if not is_timeseries_enabled():
log.warning("Timeseries not enabled")
return {"successful": False}

Expand Down
4 changes: 2 additions & 2 deletions tasks/timeseries_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
from typing import Optional

from shared.celery_config import timeseries_delete_task_name
from shared.timeseries.helpers import is_timeseries_enabled
from sqlalchemy.orm.session import Session

from app import celery_app
from database.models import Repository
from helpers.timeseries import timeseries_enabled
from services.timeseries import delete_repository_data, delete_repository_measurements
from tasks.base import BaseCodecovTask

Expand All @@ -24,7 +24,7 @@ def run_impl(
measurement_id: Optional[str] = None,
**kwargs,
):
if not timeseries_enabled():
if not is_timeseries_enabled():
log.warning("Timeseries not enabled")
return {"successful": False, "reason": "Timeseries not enabled"}

Expand Down

0 comments on commit 6f971be

Please sign in to comment.