diff --git a/app/shared/db/models.py b/app/shared/db/models.py index 0e12c7b..1736224 100644 --- a/app/shared/db/models.py +++ b/app/shared/db/models.py @@ -103,7 +103,7 @@ class Sheet(Base): author_id = Column(String, ForeignKey("users.email")) group_id = Column(String, ForeignKey("groups.id"), doc="Group ID, user must be in a group to create a sheet.") frequency = Column(String, default="daily", doc="Frequency of archiving: hourly, daily, weekly.") - # TODO: stats is not needed, is it? + # TODO: stats is not being used, consider removing stats = Column(JSON, default={}, doc="Sheet statistics like total links, total rows, ...") last_url_archived_at = Column(DateTime(timezone=True), server_default=func.now(), doc="Last time a new link was archived.") created_at = Column(DateTime(timezone=True), server_default=func.now()) diff --git a/app/shared/db/worker_crud.py b/app/shared/db/worker_crud.py index 6766786..814689a 100644 --- a/app/shared/db/worker_crud.py +++ b/app/shared/db/worker_crud.py @@ -41,15 +41,14 @@ def create_tag(db: Session, tag: str) -> models.Tag: return db_tag -def create_task(db: Session, task: schemas.ArchiveCreate, tags: list[models.Tag], urls: list[models.ArchiveUrl]) -> models.Archive: - # TODO: rename task to archive - db_task = models.Archive(id=task.id, url=task.url, result=task.result, public=task.public, author_id=task.author_id, group_id=task.group_id, sheet_id=task.sheet_id, store_until=task.store_until) - db_task.tags = tags - db_task.urls = urls - db.add(db_task) +def create_archive(db: Session, archive: schemas.ArchiveCreate, tags: list[models.Tag], urls: list[models.ArchiveUrl]) -> models.Archive: + db_archive = models.Archive(id=archive.id, url=archive.url, result=archive.result, public=archive.public, author_id=archive.author_id, group_id=archive.group_id, sheet_id=archive.sheet_id, store_until=archive.store_until) + db_archive.tags = tags + db_archive.urls = urls + db.add(db_archive) db.commit() - db.refresh(db_task) - return db_task + db.refresh(db_archive) + return db_archive def store_archived_url(db: Session, archive: schemas.ArchiveCreate) -> models.Archive: @@ -57,5 +56,5 @@ def store_archived_url(db: Session, archive: schemas.ArchiveCreate) -> models.Ar create_or_get_user(db, archive.author_id) db_tags = [create_tag(db, tag) for tag in (archive.tags or [])] # insert everything - db_task = create_task(db, task=archive, tags=db_tags, urls=archive.urls) - return db_task + db_archive = create_archive(db, archive=archive, tags=db_tags, urls=archive.urls) + return db_archive diff --git a/app/shared/schemas.py b/app/shared/schemas.py index 2a91fd7..66119f7 100644 --- a/app/shared/schemas.py +++ b/app/shared/schemas.py @@ -34,7 +34,7 @@ class TaskResult(Task): result: str -class TaskDelete(Task): +class DeleteResponse(Task): deleted: bool diff --git a/app/shared/settings.py b/app/shared/settings.py index 866b23a..d884f80 100644 --- a/app/shared/settings.py +++ b/app/shared/settings.py @@ -12,7 +12,7 @@ class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=os.environ.get("ENVIRONMENT_FILE") , env_file_encoding='utf-8', extra='ignore', str_strip_whitespace=True) # general - SERVE_LOCAL_ARCHIVE: str = "" + SERVE_LOCAL_ARCHIVE: str | None = None USER_GROUPS_FILENAME: str = "app/user-groups.yaml" # database diff --git a/app/tests/shared/db/test_worker_crud.py b/app/tests/shared/db/test_worker_crud.py index c4e6247..1098cbe 100644 --- a/app/tests/shared/db/test_worker_crud.py +++ b/app/tests/shared/db/test_worker_crud.py @@ -88,7 +88,7 @@ def test_create_task(db_session): ) # with tags and urls - nt = worker_crud.create_task(db_session, task, [models.Tag(id="tag-101")], [models.ArchiveUrl(url="https://example-0.com/0", key="media_0")]) + nt = worker_crud.create_archive(db_session, task, [models.Tag(id="tag-101")], [models.ArchiveUrl(url="https://example-0.com/0", key="media_0")]) assert nt is not None assert nt.id == "archive-id-456-101" @@ -105,7 +105,7 @@ def test_create_task(db_session): # without tags and urls task.id = "archive-id-456-102" - nt = worker_crud.create_task(db_session, task, [], []) + nt = worker_crud.create_archive(db_session, task, [], []) assert nt is not None assert nt.id == "archive-id-456-102" assert nt.url == "https://example-0.com" diff --git a/app/tests/web/db/test_crud.py b/app/tests/web/db/test_crud.py index c29cbfa..aad9d4c 100644 --- a/app/tests/web/db/test_crud.py +++ b/app/tests/web/db/test_crud.py @@ -62,78 +62,56 @@ def test_data(db_session): assert db_session.query(models.User).count() == 3 -def test_get_archive(test_data, db_session): - from app.web.config import ALLOW_ANY_EMAIL - - # each author's archives work - assert (a0 := crud.get_archive(db_session, "archive-id-456-0", authors[0])) is not None - assert a0.id == "archive-id-456-0" - assert a0.url == "https://example-0.com" - assert a0.author_id == authors[0] - assert a0.public == False - - assert crud.get_archive(db_session, "archive-id-456-1", authors[1]) is not None - assert crud.get_archive(db_session, "archive-id-456-2", authors[2]) is not None - - # ALLOW_ANY_EMAIL - assert crud.get_archive(db_session, "archive-id-456-0", ALLOW_ANY_EMAIL) is not None - assert crud.get_archive(db_session, "archive-id-456-1", ALLOW_ANY_EMAIL) is not None - - # not found - assert crud.get_archive(db_session, "archive-missing", authors[0]) is None - - # public - assert (a_public := crud.get_archive(db_session, "archive-id-456-2", authors[0])) is not None - assert a_public.public == True - - # not public - rick's - assert crud.get_archive(db_session, "archive-id-456-0", authors[1]) is None - - def test_search_archives_by_url(test_data, db_session): from app.web.config import ALLOW_ANY_EMAIL # rick's archives are private - assert len(crud.search_archives_by_url(db_session, "https://example-0.com", "rick@example.com")) == 34 - assert len(crud.search_archives_by_url(db_session, "https://example-0.com", ALLOW_ANY_EMAIL)) == 34 - assert len(crud.search_archives_by_url(db_session, "https://example-0.com", "morty@example.com")) == 0 + assert len(crud.search_archives_by_url(db_session, "https://example-0.com", "rick@example.com", True, False)) == 34 + assert len(crud.search_archives_by_url(db_session, "https://example-0.com", "rick@example.com", [], False)) == 34 + assert len(crud.search_archives_by_url(db_session, "https://example-0.com", "rick@example.com", [], True)) == 34 + assert len(crud.search_archives_by_url(db_session, "https://example-0.com", ALLOW_ANY_EMAIL, [], False)) == 34 + assert len(crud.search_archives_by_url(db_session, "https://example-0.com", ALLOW_ANY_EMAIL, True, False)) == 34 + assert len(crud.search_archives_by_url(db_session, "https://example-0.com", "morty@example.com", [], False)) == 0 + assert len(crud.search_archives_by_url(db_session, "https://example-0.com", "morty@example.com", [], True)) == 0 # morty's archives are public but half are in spaceship group - assert len(crud.search_archives_by_url(db_session, "https://example-1.com", "rick@example.com")) == 16 + assert len(crud.search_archives_by_url(db_session, "https://example-1.com", "rick@example.com", ["spaceship"], False)) == 16 + assert len(crud.search_archives_by_url(db_session, "https://example-1.com", "rick@example.com", True, False)) == 16 + assert len(crud.search_archives_by_url(db_session, "https://example-1.com", "jerry@example.com", True, True)) == 16 # jerry's archives are public - assert len(crud.search_archives_by_url(db_session, "https://example-2.com", "jerry@example.com")) == 33 - assert len(crud.search_archives_by_url(db_session, "https://example-2.com", "rick@example.com")) == 33 + assert len(crud.search_archives_by_url(db_session, "https://example-2.com", "jerry@example.com", [], True)) == 33 + assert len(crud.search_archives_by_url(db_session, "https://example-2.com", "rick@example.com", [], True)) == 33 # fuzzy search - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL)) == 100 - assert len(crud.search_archives_by_url(db_session, "https://EXAMPLE", ALLOW_ANY_EMAIL)) == 100 - assert len(crud.search_archives_by_url(db_session, "2.com", ALLOW_ANY_EMAIL)) == 33 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False)) == 100 + assert len(crud.search_archives_by_url(db_session, "https://EXAMPLE", ALLOW_ANY_EMAIL, False, False)) == 100 + assert len(crud.search_archives_by_url(db_session, "2.com", ALLOW_ANY_EMAIL, False, False)) == 33 # absolute search - assert len(crud.search_archives_by_url(db_session, "example-2.com", ALLOW_ANY_EMAIL, absolute_search=True)) == 0 - assert len(crud.search_archives_by_url(db_session, "https://example-2.com", ALLOW_ANY_EMAIL, absolute_search=True)) == 33 + assert len(crud.search_archives_by_url(db_session, "example-2.com", ALLOW_ANY_EMAIL, [], False, absolute_search=True)) == 0 + assert len(crud.search_archives_by_url(db_session, "https://example-2.com", ALLOW_ANY_EMAIL, [], False, absolute_search=True)) == 33 # archived_after - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, archived_after=datetime(2010, 1, 1))) == 100 - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, archived_after=datetime(2021, 1, 15))) == 70 - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, archived_after=datetime(2031, 1, 1))) == 0 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, True, True, archived_after=datetime(2010, 1, 1))) == 100 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, archived_after=datetime(2021, 1, 15))) == 70 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, archived_after=datetime(2031, 1, 1))) == 0 # archived before - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, archived_before=datetime(2010, 1, 1))) == 0 - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, archived_before=datetime(2021, 1, 15))) == 28 - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, archived_before=datetime(2031, 1, 1))) == 100 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, archived_before=datetime(2010, 1, 1))) == 0 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, archived_before=datetime(2021, 1, 15))) == 28 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, archived_before=datetime(2031, 1, 1))) == 100 # archived before and after - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, archived_after=datetime(2001, 1, 1), archived_before=datetime(2031, 1, 11))) == 100 - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, archived_after=datetime(2021, 1, 14), archived_before=datetime(2021, 1, 16))) == 2 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, archived_after=datetime(2001, 1, 1), archived_before=datetime(2031, 1, 11))) == 100 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, archived_after=datetime(2021, 1, 14), archived_before=datetime(2021, 1, 16))) == 2 # limit - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, limit=10)) == 10 - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, limit=-1)) == 1 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, limit=10)) == 10 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, limit=-1)) == 1 # skip - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, skip=10)) == 90 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, False, False, skip=10)) == 90 def test_search_archives_by_email(test_data, db_session): @@ -160,8 +138,8 @@ def test_search_archives_by_email(test_data, db_session): def test_max_query_limit(test_data, db_session): from app.web.config import ALLOW_ANY_EMAIL - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL)) == 25 - assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, limit=1000)) == 25 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, [], False)) == 25 + assert len(crud.search_archives_by_url(db_session, "https://example", ALLOW_ANY_EMAIL, True, True, limit=1000)) == 25 assert len(crud.search_archives_by_email(db_session, "rick@example.com")) == 25 assert len(crud.search_archives_by_email(db_session, "rick@example.com", limit=1000)) == 25 @@ -169,18 +147,18 @@ def test_max_query_limit(test_data, db_session): def test_soft_delete(test_data, db_session): # none deleted yet - assert crud.get_archive(db_session, "archive-id-456-0", "rick@example.com") is not None + db_session.query(models.Archive).filter(models.Archive.id == "archive-id-456-0").first() is not None assert db_session.query(models.Archive).filter(models.Archive.deleted == True).count() == 0 # delete - assert crud.soft_delete_task(db_session, "archive-id-456-0", "rick@example.com") == True + assert crud.soft_delete_archive(db_session, "archive-id-456-0", "rick@example.com") == True # ensure soft delete assert db_session.query(models.Archive).filter(models.Archive.deleted == True).count() == 1 - assert crud.get_archive(db_session, "archive-id-456-0", "rick@example.com") is None + db_session.query(models.Archive).filter(models.Archive.id == "archive-id-456-0").first() is None # already deleted - assert crud.soft_delete_task(db_session, "archive-id-456-0", "rick@example.com") == False + assert crud.soft_delete_archive(db_session, "archive-id-456-0", "rick@example.com") == False def test_count_archives(test_data, db_session): diff --git a/app/tests/web/endpoints/test_interoperability.py b/app/tests/web/endpoints/test_interoperability.py index edf8c0b..14cd245 100644 --- a/app/tests/web/endpoints/test_interoperability.py +++ b/app/tests/web/endpoints/test_interoperability.py @@ -2,6 +2,7 @@ from datetime import datetime import json from unittest.mock import MagicMock, patch +from app.shared.db import models from app.web.config import ALLOW_ANY_EMAIL from app.web.db import crud @@ -22,7 +23,7 @@ def test_submit_manual_archive(m1, client_with_token, db_session): assert r.status_code == 201 assert "id" in r.json() - inserted = crud.get_archive(db_session, r.json()["id"], ALLOW_ANY_EMAIL) + inserted = db_session.query(models.Archive).filter(models.Archive.id == r.json()["id"]).first() assert inserted.url == "http://example.com" assert inserted.group_id == "spaceship" assert inserted.author_id == "jerry@gmail.com" diff --git a/app/tests/web/endpoints/test_url.py b/app/tests/web/endpoints/test_url.py index b8bc15b..4a6f342 100644 --- a/app/tests/web/endpoints/test_url.py +++ b/app/tests/web/endpoints/test_url.py @@ -133,8 +133,7 @@ def test_search_by_url(client_with_auth, client_with_token, db_session): from app.shared import schemas from app.shared.db import worker_crud for i in range(11): - #TODO: fix as this method is gone to shared.db - worker_crud.create_task(db_session, ArchiveCreate(id=f"url-456-{i}", url="https://example.com" if i < 10 else "https://something-else.com", result={}, public=True, author_id="rick@example.com"), [], []) + worker_crud.create_archive(db_session, ArchiveCreate(id=f"url-456-{i}", url="https://example.com" if i < 10 else "https://something-else.com", result={}, public=True, author_id="rick@example.com"), [], []) # NB: this insertion is too fast for the ordering to be correct as they are within the same second response = client_with_auth.get("/url/search?url=https://example.com") @@ -187,7 +186,7 @@ def test_delete_task(client_with_auth, db_session): assert response.json() == {"id": "delete-123-456-789", "deleted": False} from app.shared.db import worker_crud - worker_crud.create_task(db_session, ArchiveCreate(id="delete-123-456-789", url="https://example.com", result={}, public=True, author_id="morty@example.com"), [], []) + worker_crud.create_archive(db_session, ArchiveCreate(id="delete-123-456-789", url="https://example.com", result={}, public=True, author_id="morty@example.com"), [], []) response = client_with_auth.delete("/url/delete-123-456-789") assert response.status_code == 200 diff --git a/app/tests/web/test_main.py b/app/tests/web/test_main.py index 8cee578..f77d368 100644 --- a/app/tests/web/test_main.py +++ b/app/tests/web/test_main.py @@ -17,7 +17,7 @@ def test_alembic(db_session): alembic.config.main(argv=['--raiseerr', 'upgrade', 'head']) alembic.config.main(argv=['--raiseerr', 'downgrade', 'base']) -@patch("app.web.endpoints.url.crud.soft_delete_task", side_effect=Exception('mocked error')) +@patch("app.web.endpoints.url.crud.soft_delete_archive", side_effect=Exception('mocked error')) def test_logging_middleware(m1, client_with_auth): from app.web.utils.metrics import EXCEPTION_COUNTER assert len(EXCEPTION_COUNTER.collect()[0].samples) == 0 diff --git a/app/web/db/crud.py b/app/web/db/crud.py index fa159e8..c16b09a 100644 --- a/app/web/db/crud.py +++ b/app/web/db/crud.py @@ -32,20 +32,18 @@ def base_query(db: Session): .options(load_only(models.Archive.id, models.Archive.created_at, models.Archive.url, models.Archive.result, models.Archive.store_until)) -def get_archive(db: Session, id: str, email: str): - query = base_query(db).filter(models.Archive.id == id) - if email != ALLOW_ANY_EMAIL: - groups = get_user_group_names(db ,email) - query = query.filter(or_(models.Archive.public == True, models.Archive.author_id == email, models.Archive.group_id.in_(groups))) - return query.first() - - -def search_archives_by_url(db: Session, url: str, email: str, skip: int = 0, limit: int = 100, archived_after: datetime = None, archived_before: datetime = None, absolute_search: bool = False) -> list[models.Archive]: - # searches for partial URLs, if email is * no ownership filtering happens +def search_archives_by_url(db: Session, url: str, email: str, read_groups: bool | set[str], read_public: bool, skip: int = 0, limit: int = 100, archived_after: datetime = None, archived_before: datetime = None, absolute_search: bool = False) -> list[models.Archive]: + # searches for partial URLs, if email is * no ownership (or read/read_public) filtering happens query = base_query(db) if email != ALLOW_ANY_EMAIL: - groups = get_user_group_names(db, email) - query = query.filter(or_(models.Archive.public == True, models.Archive.author_id == email, models.Archive.group_id.in_(groups))) + or_filters = [models.Archive.author_id == email] + if read_public: + or_filters.append(models.Archive.public == True) + if read_groups == True: + or_filters.append(models.Archive.group_id.isnot(None)) + else: + or_filters.append(models.Archive.group_id.in_(read_groups)) + query = query.filter(or_(*or_filters)) if absolute_search: query = query.filter(models.Archive.url == url) else: @@ -61,13 +59,13 @@ def search_archives_by_email(db: Session, email: str, skip: int = 0, limit: int return base_query(db).filter(models.Archive.author_id == email).order_by(models.Archive.created_at.desc()).offset(skip).limit(get_limit(limit)).all() -def soft_delete_task(db: Session, task_id: str, email: str) -> bool: +def soft_delete_archive(db: Session, id: str, email: str) -> bool: # TODO: implement hard-delete with cronjob that deletes from S3 - db_task = db.query(models.Archive).filter(models.Archive.id == task_id, models.Archive.author_id == email, models.Archive.deleted == False).first() - if db_task: - db_task.deleted = True + db_archive = db.query(models.Archive).filter(models.Archive.id == id, models.Archive.author_id == email, models.Archive.deleted == False).first() + if db_archive: + db_archive.deleted = True db.commit() - return db_task is not None + return db_archive is not None def count_archives(db: Session): @@ -121,7 +119,7 @@ def get_user_group_names(db: Session, email: str) -> list[str]: """ given an email retrieves the user groups from the DB and then the email-domain groups from a global variable, the email does not need to belong to an existing user. """ - #TODO: the read: [group1, group2] permissions don't currently work + # TODO: the read: [group1, group2] permissions don't currently work if not email or not len(email) or "@" not in email: return [] # get user groups @@ -178,8 +176,8 @@ def upsert_user_groups(db: Session): reads the user_groups yaml file and inserts any new users, groups, along with new participation of users in groups """ - logger.debug("Updating user-groups configuration.") filename = get_settings().USER_GROUPS_FILENAME + logger.debug(f"Updating user-groups configuration with file {filename}.") ug = UserGroups(filename) diff --git a/app/web/endpoints/sheet.py b/app/web/endpoints/sheet.py index 9177668..7848b5e 100644 --- a/app/web/endpoints/sheet.py +++ b/app/web/endpoints/sheet.py @@ -51,7 +51,7 @@ def delete_sheet( id: str, user: UserState = Depends(get_user_state), db: Session = Depends(get_db_dependency), -) -> schemas.TaskDelete: +) -> schemas.DeleteResponse: return JSONResponse({ "id": id, "deleted": crud.delete_sheet(db, id, user.email) diff --git a/app/web/endpoints/url.py b/app/web/endpoints/url.py index c0c1284..c237893 100644 --- a/app/web/endpoints/url.py +++ b/app/web/endpoints/url.py @@ -61,22 +61,24 @@ def search_by_url( email: str = Depends(get_token_or_user_auth) ) -> list[schemas.ArchiveResult]: + read_groups, read_public = False, False if email != ALLOW_ANY_EMAIL: user = UserState(db, email) if not user.read and not user.read_public: raise HTTPException(status_code=403, detail="User does not have read access.") - - return crud.search_archives_by_url(db, url.strip(), email, skip=skip, limit=limit, archived_after=archived_after, archived_before=archived_before) + read_groups = user.read + read_public = user.read_public + return crud.search_archives_by_url(db, url.strip(), email, read_groups, read_public, skip=skip, limit=limit, archived_after=archived_after, archived_before=archived_before) @url_router.delete("/{id}", summary="Delete a single URL archive by id.") -def delete_task( +def delete_archive( id:str, user: UserState = Depends(get_user_state), db: Session = Depends(get_db_dependency) -) -> schemas.TaskDelete: +) -> schemas.DeleteResponse: logger.info(f"deleting url archive task {id} request by {user.email}") return JSONResponse({ "id": id, - "deleted": crud.soft_delete_task(db, id, user.email) + "deleted": crud.soft_delete_archive(db, id, user.email) }) diff --git a/app/web/events.py b/app/web/events.py index 88bb18f..625731a 100644 --- a/app/web/events.py +++ b/app/web/events.py @@ -15,6 +15,7 @@ from app.shared import schemas from app.shared.settings import get_settings from app.shared.task_messaging import get_celery from app.web.db import crud +from app.web.middleware import increase_exceptions_counter from app.web.utils.metrics import measure_regular_metrics, redis_subscribe_worker_exceptions celery = get_celery() @@ -60,17 +61,17 @@ async def lifespan(app: FastAPI): # CRON JOBS -@repeat_every(seconds=get_settings().REPEAT_COUNT_METRICS_SECONDS, on_exception=logger.error) +@repeat_every(seconds=get_settings().REPEAT_COUNT_METRICS_SECONDS, on_exception=increase_exceptions_counter) async def repeat_measure_regular_metrics(): await measure_regular_metrics(get_settings().DATABASE_PATH, get_settings().REPEAT_COUNT_METRICS_SECONDS) -@repeat_every(seconds=60, wait_first=120, on_exception=logger.error) +@repeat_every(seconds=60, wait_first=120, on_exception=increase_exceptions_counter) async def archive_hourly_sheets_cronjob(): await archive_sheets_cronjob("hourly", 60, datetime.datetime.now().minute) -@repeat_every(seconds=3600, wait_first=120, on_exception=logger.error) +@repeat_every(seconds=3600, wait_first=120, on_exception=increase_exceptions_counter) async def archive_daily_sheets_cronjob(): await archive_sheets_cronjob("daily", 24, datetime.datetime.now().hour) @@ -92,7 +93,7 @@ async def archive_sheets_cronjob(frequency: str, interval: int, current_time_uni DELETE_WINDOW = get_settings().DELETE_SCHEDULED_ARCHIVES_CHECK_EVERY_N_DAYS * 24 * 60 * 60 -@repeat_every(seconds=DELETE_WINDOW, wait_first=180, on_exception=logger.error) +@repeat_every(seconds=DELETE_WINDOW, wait_first=180, on_exception=increase_exceptions_counter) async def notify_about_expired_archives(): notify_from = datetime.datetime.now() + datetime.timedelta(days=get_settings().DELETE_SCHEDULED_ARCHIVES_CHECK_EVERY_N_DAYS) async with get_db_async() as db: @@ -135,7 +136,7 @@ async def notify_about_expired_archives(): asyncio.create_task(delete_expired_archives()) -@repeat_every(max_repetitions=1, wait_first=10, seconds=0, on_exception=logger.error) +@repeat_every(max_repetitions=1, wait_first=10, seconds=0, on_exception=increase_exceptions_counter) async def delete_expired_archives(): async with get_db_async() as db: count_deleted = await crud.soft_delete_expired_archives(db) @@ -143,7 +144,7 @@ async def delete_expired_archives(): logger.debug(f"[CRON] Deleted {count_deleted} archives.") -@repeat_every(seconds=86400, wait_first=150, on_exception=logger.error) +@repeat_every(seconds=86400, wait_first=150, on_exception=increase_exceptions_counter) async def delete_stale_sheets(): STALE_DAYS = get_settings().DELETE_STALE_SHEETS_DAYS logger.debug(f"[CRON] Deleting stale sheets older than {STALE_DAYS} days.") diff --git a/app/web/main.py b/app/web/main.py index cc7774c..ff2266e 100644 --- a/app/web/main.py +++ b/app/web/main.py @@ -49,12 +49,12 @@ def app_factory(settings = get_settings()): # prometheus exposed in /metrics with authentication Instrumentator(should_group_status_codes=False, excluded_handlers=["/metrics", "/health", "/openapi.json", "/favicon.ico"]).instrument(app).expose(app, dependencies=[Depends(token_api_key_auth)]) - # TODO: recheck this for security, currently only needed for when local_storage is used in development - local_dir = settings.SERVE_LOCAL_ARCHIVE - if not os.path.isdir(local_dir) and os.path.isdir(local_dir.replace("/app", ".")): - local_dir = local_dir.replace("/app", ".") - if len(settings.SERVE_LOCAL_ARCHIVE) > 1 and os.path.isdir(local_dir): - logger.warning(f"MOUNTing local archive {settings.SERVE_LOCAL_ARCHIVE}") - app.mount(settings.SERVE_LOCAL_ARCHIVE, StaticFiles(directory=local_dir), name=settings.SERVE_LOCAL_ARCHIVE) + if settings.SERVE_LOCAL_ARCHIVE: + local_dir = settings.SERVE_LOCAL_ARCHIVE + if not os.path.isdir(local_dir) and os.path.isdir(local_dir.replace("/app", ".")): + local_dir = local_dir.replace("/app", ".") + if len(settings.SERVE_LOCAL_ARCHIVE) > 1 and os.path.isdir(local_dir): + logger.warning(f"MOUNTing local archive, use this in development only {settings.SERVE_LOCAL_ARCHIVE}") + app.mount(settings.SERVE_LOCAL_ARCHIVE, StaticFiles(directory=local_dir), name=settings.SERVE_LOCAL_ARCHIVE) return app \ No newline at end of file diff --git a/app/web/middleware.py b/app/web/middleware.py index 227a620..52da626 100644 --- a/app/web/middleware.py +++ b/app/web/middleware.py @@ -1,4 +1,5 @@ +import traceback from loguru import logger from fastapi import Request from app.shared.log import log_error @@ -13,7 +14,18 @@ async def logging_middleware(request: Request, call_next): logger.info(f"{request.client.host}:{request.client.port} {request.method} {request.url._url} - HTTP {response.status_code}") return response except Exception as e: - EXCEPTION_COUNTER.labels(type=e.__class__.__name__).inc() - logger.info(f"{request.client.host}:{request.client.port} {request.method} {request.url._url} - {e.__class__.__name__} {e}") - log_error(e) + location = f"{request.method} {request.url._url}" + await increase_exceptions_counter(e, location) + logger.info(f"{request.client.host}:{request.client.port} {location} - {e.__class__.__name__} {e}") raise e + +async def increase_exceptions_counter(e: Exception, location:str="cronjob"): + if location == "cronjob": + try: + last_trace = traceback.extract_tb(e.__traceback__)[-1] + _file, _line, func_name, _text = last_trace + location = func_name + except Exception as e: + logger.error(f"Unable to get function name from cronjob exception traceback: {e}") + EXCEPTION_COUNTER.labels(type=e.__class__.__name__, location=location).inc() + log_error(e) \ No newline at end of file diff --git a/app/web/utils/metrics.py b/app/web/utils/metrics.py index 64aaf9c..a885b9a 100644 --- a/app/web/utils/metrics.py +++ b/app/web/utils/metrics.py @@ -14,7 +14,7 @@ from app.shared.task_messaging import get_redis EXCEPTION_COUNTER = Counter( "exceptions", "Number of times a certain exception has occurred.", - labelnames=["type"] + labelnames=["type", "location"] ) WORKER_EXCEPTION = Counter( "worker_exceptions_total", diff --git a/app/worker/main.py b/app/worker/main.py index 115ea9a..a9a10c6 100644 --- a/app/worker/main.py +++ b/app/worker/main.py @@ -27,14 +27,14 @@ USER_GROUPS_FILENAME = settings.USER_GROUPS_FILENAME logger.remove = lambda x: print(f"logger.remove({x})") # TODO: after release, as it requires updating past entries with sheet_id where tag is used, drop tags -@celery.task(name="create_archive_task", bind=True, autoretry_for=(Exception,), retry_backoff=True, retry_kwargs={'max_retries': 0}) +@celery.task(name="create_archive_task", bind=True, autoretry_for=(Exception,), retry_backoff=True, retry_kwargs={'max_retries': 1}) def create_archive_task(self, archive_json: str): archive = schemas.ArchiveCreate.model_validate_json(archive_json) # call auto-archiver args = get_orchestrator_args(archive.group_id, False, [archive.url]) # args = get_orchestrator_args(archive.group_id, False, [archive.url, "--extractors", "generic_extractor"]) - logger.error(args) + logger.debug(args) try: result = next(ArchivingOrchestrator().run(args), None) except SystemExit as e: @@ -61,6 +61,7 @@ def create_sheet_task(self, sheet_json: str): logger.info(f"[queue={queue_name}] SHEET START {sheet=}") args = get_orchestrator_args(sheet.group_id, True, ["--gsheet_feeder.sheet_id", sheet.sheet_id]) + logger.info(f"[queue={queue_name}] {args=}") stats = {"archived": 0, "failed": 0, "errors": []} try: @@ -116,9 +117,9 @@ def get_orchestrator_args(group_id: str, orchestrator_for_sheet: bool, cli_args: def insert_result_into_db(archive: schemas.ArchiveCreate) -> str: with get_db() as session: - db_task = worker_crud.store_archived_url(session, archive) - logger.debug(f"[ARCHIVE STORED] {db_task.author_id} {db_task.url}") - return db_task.id + db_archive = worker_crud.store_archived_url(session, archive) + logger.debug(f"[ARCHIVE STORED] {db_archive.author_id} {db_archive.url}") + return db_archive.id def get_store_until(group_id: str) -> datetime.datetime: