Skip to content

Commit 46086a9

Browse files
committed
Delay initial processing and retry via Cloud Tasks when the revision is not yet public
1 parent ed0804f commit 46086a9

5 files changed

Lines changed: 48 additions & 3 deletions

File tree

services/reviewhelper-api/app/review_processor.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import logging
22
from collections.abc import AsyncIterator
3+
from datetime import UTC, datetime, timedelta
34
from functools import cache
45
from typing import Collection, Iterable
56

@@ -13,11 +14,20 @@
1314

1415
logger = logging.getLogger(__name__)
1516

17+
VISIBILITY_TIMEOUT = timedelta(minutes=5)
18+
1619

1720
class ReviewProcessingError(Exception):
1821
"""Custom exception for permanent errors during review processing that should not trigger retries."""
1922

2023

24+
class RevisionNotYetPublicError(Exception):
25+
"""The revision is not yet public but was recently created.
26+
27+
This is a transient error — Cloud Tasks should retry after backoff.
28+
"""
29+
30+
2131
@cache
2232
def get_code_review_tool():
2333
from bugbug.tools.code_review import CodeReviewTool
@@ -50,6 +60,13 @@ async def process_review(
5060
raise ValueError(f"Unsupported platform: {review_request.platform}")
5161

5262
if not patch.is_accessible() or not patch.is_public():
63+
age = datetime.now(UTC) - review_request.created_at
64+
if age < VISIBILITY_TIMEOUT:
65+
raise RevisionNotYetPublicError(
66+
f"Revision D{review_request.revision_id} is not public. "
67+
"But the review request was created recently, so this may be a visibility delay."
68+
)
69+
5370
raise ReviewProcessingError(
5471
"Unable to access the revision. This may be because "
5572
"the revision is private or has restricted visibility."

services/reviewhelper-api/app/routers/internal.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from app.enums import ReviewStatus
1414
from app.review_processor import (
1515
ReviewProcessingError,
16+
RevisionNotYetPublicError,
1617
process_review,
1718
submit_review_to_platform,
1819
)
@@ -131,6 +132,11 @@ async def _set_retry_pending(db: AsyncSession):
131132
# the connection.
132133
db.add(review_request)
133134

135+
except RevisionNotYetPublicError as e:
136+
logger.warning("Review request %s: %s", review_request_id, e)
137+
review_request.status = ReviewStatus.RETRY_PENDING
138+
await db.commit()
139+
return Response(status_code=status.HTTP_409_CONFLICT)
134140
except ReviewProcessingError as e:
135141
review_request.error = str(e)
136142
review_request.status = ReviewStatus.FAILED

services/reviewhelper-api/app/routers/request.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import logging
2+
from datetime import timedelta
23
from typing import Annotated
34

45
from fastapi import APIRouter, Depends, status
@@ -15,6 +16,12 @@
1516

1617
logger = logging.getLogger(__name__)
1718

19+
# For newly-created revisions, delay the task to give phab-bot time to
20+
# update visibility before we attempt processing. If the delay has
21+
# already elapsed, Cloud Tasks dispatches immediately.
22+
INITIAL_TASK_DELAY = timedelta(seconds=30)
23+
24+
1825
router = APIRouter(
1926
tags=["request"],
2027
dependencies=[Depends(verify_external_api_key)],
@@ -75,8 +82,17 @@ async def create_or_get_review_request(
7582
db.add(review_request)
7683
await db.commit()
7784

85+
schedule_time = (
86+
request.revision_created_at + INITIAL_TASK_DELAY
87+
# NOTE: This is a check if for backwards compatibility with older
88+
# version of Phabricator, and can be dropped once
89+
# https://github.com/mozilla-conduit/phabricator/pull/84 is deployed
90+
if request.revision_created_at is not None
91+
else None
92+
)
93+
7894
# Queue task for processing
79-
await create_review_task(review_request.id)
95+
await create_review_task(review_request.id, schedule_time=schedule_time)
8096

8197
return JSONResponse(
8298
{

services/reviewhelper-api/app/schemas/review_request.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from pydantic import BaseModel
1+
from pydantic import BaseModel, PastDatetime
22

33
from app.enums import ReviewStatus
44
from app.schemas.base import UserActionBase
@@ -9,6 +9,7 @@ class ReviewRequestCreate(UserActionBase):
99

1010
revision_id: int
1111
diff_id: int
12+
revision_created_at: PastDatetime | None = None
1213

1314

1415
class ReviewRequestResponse(BaseModel):

services/reviewhelper-api/app/tasks.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import logging
2+
from datetime import datetime
23
from functools import cache
34

45
from google.cloud.tasks_v2 import (
@@ -19,11 +20,14 @@ def _get_tasks_client():
1920
return CloudTasksAsyncClient()
2021

2122

22-
async def create_review_task(review_request_id: int):
23+
async def create_review_task(
24+
review_request_id: int, schedule_time: datetime | None = None
25+
):
2326
"""Create a Cloud Task to process a review request.
2427
2528
Args:
2629
review_request_id: The ID of the review request to process.
30+
schedule_time: Optional time to delay task dispatch until.
2731
"""
2832
client = _get_tasks_client()
2933

@@ -44,6 +48,7 @@ async def create_review_task(review_request_id: int):
4448
},
4549
),
4650
dispatch_deadline=Duration(seconds=30 * 60),
51+
schedule_time=schedule_time,
4752
)
4853

4954
response = await client.create_task(parent=parent, task=task)

0 commit comments

Comments
 (0)