Skip to content

Commit 9da8d34

Browse files
irenelagnoclaude
andcommitted
docs(geo-brand-presence): correct DRS migration plan based on code review feedback
- _create_synthetic_job() lives in spacecat_resolver.py, not fargate_trigger.py; must be ported (not reused) as part of the DRS PR - excel_ingestion.py cannot be fully deleted; parse_excel_to_rows is used by the Fargate prompt loader — only unused functions should be removed - Rewrote Repo 1 section as a high-level summary without specific file paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent e797d81 commit 9da8d34

1 file changed

Lines changed: 29 additions & 80 deletions

File tree

docs/plans/2026-04-22-geo-brand-presence-drs-sns-migration.md

Lines changed: 29 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -45,95 +45,44 @@ SpaceCat refresh handler
4545

4646
## Repo 1: adobe-rnd/llmo-data-retrieval-service (PR A)
4747

48-
### Goal
49-
- Extend `fargate_trigger.py` to handle direct SpaceCat SNS messages (no pre-existing job)
50-
- Delete the now-redundant `POST /brand-presence/analyze` HTTP endpoint
51-
- Update the design document
52-
53-
### `src/pipelines/brand_presence/handlers/fargate_trigger.py`
54-
55-
When `provider_id == "external_spacecat"` and the job is not found in DynamoDB,
56-
create the synthetic parent job inline using metadata from the SNS notification:
57-
58-
```python
59-
job = await self._get_job(notification.job_id)
60-
if job is None and notification.provider_id == "external_spacecat":
61-
job = await self._create_synthetic_job(
62-
job_id=notification.job_id,
63-
provider_id="external_spacecat",
64-
result_location=notification.result_location,
65-
site_id=notification.metadata.site_id,
66-
brand=notification.metadata.brand,
67-
ims_org_id=notification.metadata.ims_org_id,
68-
week=notification.week,
69-
year=notification.year,
70-
run_frequency=notification.metadata.run_frequency,
71-
web_search_provider=notification.metadata.web_search_provider,
72-
config_version=notification.metadata.config_version,
73-
)
74-
elif job is None:
75-
log.warning("Job not found and not external_spacecat — skipping")
76-
return
77-
```
48+
### What needs to happen
7849

79-
`_create_synthetic_job()` already exists (used by `SpaceCatResolver`). Reuse it.
50+
**1. Handle direct SpaceCat SNS in the Fargate trigger**
8051

81-
### `src/common/models/notifications.py`
52+
The Fargate trigger handler must be extended to process `provider_id="external_spacecat"` SNS
53+
messages that arrive without a pre-existing DynamoDB job. When such a message is received and
54+
no job record is found, a synthetic job must be created inline from the SNS notification metadata
55+
(site_id, week, year, run_frequency, web_search_provider, config_version, result_location).
8256

83-
Add optional fields to `JobNotificationMetadata`:
57+
Note: the synthetic-job creation logic currently lives in `spacecat_resolver.py`, not in
58+
`fargate_trigger.py`. It must be **ported** into the Fargate trigger handler as part of this PR —
59+
it cannot simply be called from its current location.
8460

85-
```python
86-
brand: Optional[str] = None
87-
ims_org_id: Optional[str] = None
88-
web_search_provider: Optional[str] = None
89-
config_version: Optional[str] = None
90-
run_frequency: Optional[str] = None # "daily" | "weekly"
91-
```
61+
The notification model also needs to accept the new optional metadata fields
62+
(brand, ims_org_id, web_search_provider, config_version, run_frequency) that SpaceCat will
63+
include in the SNS message.
9264

93-
### Delete: `POST /brand-presence/analyze` HTTP endpoint
94-
95-
Since integration now goes through S3 + SNS, the HTTP Lambda is redundant.
96-
97-
Files to delete:
98-
- `src/pipelines/brand_presence/handlers/brand_presence_analyze.py`
99-
- `src/pipelines/brand_presence/resolvers/spacecat_resolver.py`
100-
- `src/pipelines/brand_presence/services/excel_ingestion.py` (verify not used elsewhere first)
101-
- `tests/pipelines/brand_presence/handlers/test_brand_presence_analyze.py`
102-
- `tests/pipelines/brand_presence/resolvers/test_spacecat_resolver.py`
103-
104-
CDK changes:
105-
- `api_gateway_nested_stack.py` — remove `POST /sites/{siteId}/brand-presence/analyze` route
106-
- `brand_presence_nested_stack.py` — remove `AnalyzeBrandPresenceFunction` Lambda
107-
- `drs_v2_stack.py` — remove `analyze_brand_presence_function_arn` references
108-
109-
### DRS S3 Bucket Policy
110-
111-
Add cross-account write permission for SpaceCat's Lambda execution role:
112-
113-
```python
114-
self.brand_presence_bucket.add_to_resource_policy(
115-
iam.PolicyStatement(
116-
effect=iam.Effect.ALLOW,
117-
principals=[iam.ArnPrincipal(spacecat_lambda_role_arn)],
118-
actions=["s3:PutObject"],
119-
resources=[
120-
self.brand_presence_bucket.arn_for_objects("external/spacecat/*")
121-
],
122-
)
123-
)
124-
```
65+
**2. Remove the `POST /brand-presence/analyze` HTTP endpoint**
12566

126-
### Update Design Document: `docs/design/reanalyze-brand-presence.md`
67+
Since the integration path is now S3 + SNS, the HTTP Lambda that previously accepted Excel
68+
uploads from SpaceCat is redundant and should be deleted. This includes the Lambda handler,
69+
its resolver, the CDK resources wiring it up, and its tests.
12770

128-
Add new section describing Path B (revised) — Direct S3 + SNS, note removal of
129-
`POST /brand-presence/analyze` HTTP endpoint and `SpaceCatResolver`.
130-
Document that `JobNotificationMetadata` now carries additional fields for direct SpaceCat publishes.
71+
Caution: `excel_ingestion.py` (or equivalent Excel-parsing service) cannot be fully deleted —
72+
`parse_excel_to_rows` (or similar) is imported by the Fargate prompt loader. Only remove
73+
functions that are exclusively used by the deleted HTTP endpoint; leave anything used by
74+
Fargate in place.
13175

132-
### Tests
76+
**3. Grant cross-account S3 write access**
77+
78+
The DRS brand-presence S3 bucket policy must allow SpaceCat's Lambda execution role to
79+
`s3:PutObject` under the `external/spacecat/*` prefix.
80+
81+
**4. Update the design document**
13382

134-
- `test_fargate_trigger.py`: add tests for `external_spacecat` SNS with no pre-existing job
135-
`_create_synthetic_job` called, Fargate launched; existing job path unchanged
136-
- Delete: `test_brand_presence_analyze.py`, `test_spacecat_resolver.py`
83+
`docs/design/reanalyze-brand-presence.md` should be updated to describe the new Path B
84+
(Direct S3 + SNS), document removal of the HTTP endpoint, and note the new metadata fields
85+
carried in the SNS notification.
13786

13887
---
13988

0 commit comments

Comments
 (0)