Skip to content

Commit 9985ca8

Browse files
committed
fix: address simple mailing list PR review comments (CM-1318)
Signed-off-by: Uroš Marolt <uros@marolt.me>
1 parent f521e7a commit 9985ca8

6 files changed

Lines changed: 34 additions & 27 deletions

File tree

.github/workflows/backend-lint.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ jobs:
102102
if: steps.changes.outputs.python_changed == 'true'
103103
run: |
104104
uv run ruff check src/ --output-format=github
105+
uv run ruff format --check src/
105106
106107
lint-python-mailing-list-integration:
107108
runs-on: ubuntu-latest
@@ -146,6 +147,10 @@ jobs:
146147
uv run ruff check src/ --output-format=github
147148
uv run ruff format --check src/
148149
150+
- name: Run tests
151+
if: steps.changes.outputs.python_changed == 'true'
152+
run: uv run pytest src/test/ -v
153+
149154
- name: Skip Python checks
150155
if: steps.changes.outputs.python_changed == 'false'
151156
run: echo "⏭️ No Python files changed, skipping linting checks"
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
-- Mailing list (Collaboration)
2-
INSERT INTO "activityTypes" ("activityType", platform, "isCodeContribution", "isCollaboration", description) VALUES
3-
('message', 'mailinglist', false, true, 'Sent a message to a mailing list');
2+
INSERT INTO "activityTypes" ("activityType", platform, "isCodeContribution", "isCollaboration", description, "label") VALUES
3+
('message', 'mailinglist', false, true, 'Sent a message to a mailing list', 'Sent a message');

services/apps/mailing_list_integration/README.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ tracker referenced in the PR for build progress.
1313
process, mirrors the list via `public-inbox-clone`/`public-inbox-fetch`,
1414
parses new messages, writes activities to `integration.results`, and emits
1515
Kafka messages to the `data-sink-worker` topic — same plumbing as
16-
git_integration, with `platform=groupsio`.
16+
git_integration, with `platform=mailinglist`.
1717
- `services/mirror/` wraps the external `public-inbox` CLI (Perl tool,
1818
installed system-wide, not vendored).
1919
- `services/parse/` is the ported email parser (originally `noteren.py`),
@@ -43,7 +43,7 @@ There is no UI yet — a list is onboarded by inserting its rows directly.
4343
psql "$CROWD_DB_WRITE_URI" -f dev/seed.sql
4444
```
4545

46-
It creates a `public.integrations` row (`platform='groupsio'`), a
46+
It creates a `public.integrations` row (`platform='mailinglist'`), a
4747
`mailinglist.lists` row pointing at an existing segment, and a
4848
`mailinglist."listProcessing"` row in state `pending` — which the worker's
4949
`acquire_onboarding_list()` will pick up on its next poll. Edit the `name`
@@ -59,17 +59,18 @@ and `"sourceUrl"` values in the file to onboard a different lore list.
5959
- inserts rows into `integration.results` with `state='pending'` and
6060
`data.type='activity'`,
6161
- emits one Kafka message per result (`process_integration_result`,
62-
`platform=groupsio`),
62+
`platform=mailinglist`),
6363
- advances `mailinglist."listProcessing"."lastProcessedHeads"` and sets
6464
`state='completed'`.
6565
4. Confirm `data_sink_worker` consumes the message, resolves the integration's
66-
`platform='groupsio'`, and creates the member (email-based verified
66+
`platform='mailinglist'`, and creates the member (email-based verified
6767
`username`+`email` identities) and activity.
6868
5. `GET http://localhost:8086/` returns the FastAPI health check.
6969

7070
## Out of scope (CM-1318)
7171

72-
- Onboarding UI/backend to create `integrations` + `mailinglist.lists` rows
73-
(this is what `dev/seed.sql` stands in for).
72+
- Onboarding UI to create `integrations` + `mailinglist.lists` rows (the
73+
`mailing-list-connect` backend endpoint exists; `dev/seed.sql` remains
74+
useful for onboarding lists without going through the UI).
7475
- Whether `member_join`/`member_leave` activities are derivable from lore, or
7576
only `message` initially.

services/apps/mailing_list_integration/src/crowdmail/server.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,13 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
2424
yield
2525
finally:
2626
await worker.shutdown() # stopping worker to process new lists
27-
try:
28-
await asyncio.wait_for(worker_task, timeout=WORKER_SHUTDOWN_TIMEOUT_SEC)
29-
logger.info("Worker shutdown complete")
30-
except TimeoutError:
31-
logger.warning("Worker shutdown timeout, forcing cancellation")
32-
worker_task.cancel()
27+
if worker_task is not None:
28+
try:
29+
await asyncio.wait_for(worker_task, timeout=WORKER_SHUTDOWN_TIMEOUT_SEC)
30+
logger.info("Worker shutdown complete")
31+
except TimeoutError:
32+
logger.warning("Worker shutdown timeout, forcing cancellation")
33+
worker_task.cancel()
3334

3435

3536
app = FastAPI(lifespan=lifespan)

services/apps/mailing_list_integration/src/crowdmail/services/parse/noteren.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import sys
2121

2222
from crowdmail.enums import ActivityType
23+
from crowdmail.errors import CommandExecutionError
2324

2425
# We want to always be using utf-8
2526
email.charset.add_charset("utf-8", None)
@@ -68,8 +69,9 @@ def get_email_from_git(git_dir: str, git_id: str) -> tuple[bytes, str]:
6869
git_blob_id = out.decode().rstrip() # strip the trailing \n
6970

7071
if git_blob_id == "":
71-
logging.error('git commit id %s was not found in the git repo "%s"', git_id, git_dir)
72-
sys.exit(1)
72+
msg = f'git commit id {git_id} was not found in the git repo "{git_dir}"'
73+
logging.error(msg)
74+
raise CommandExecutionError(msg)
7375

7476
logging.debug("git_blob_id =%s", git_blob_id)
7577

@@ -79,12 +81,9 @@ def get_email_from_git(git_dir: str, git_id: str) -> tuple[bytes, str]:
7981
# decoding itself, and decoding here would corrupt non-utf-8 parts.
8082
blob = out
8183
else:
82-
logging.error(
83-
'Unable to retrieve git blob %s from the git repo "%s"',
84-
git_blob_id,
85-
git_dir,
86-
)
87-
sys.exit(1)
84+
msg = f'Unable to retrieve git blob {git_blob_id} from the git repo "{git_dir}"'
85+
logging.error(msg)
86+
raise CommandExecutionError(msg)
8887

8988
return blob, git_blob_id
9089

@@ -260,7 +259,7 @@ def parse_email(
260259
# of emitting a malformed timestamp.
261260
if not 1970 <= date_dt.year <= 9999:
262261
raise ValueError(f"implausible year {date_dt.year}")
263-
# add a fake set of microseconds just to make date parsers on the injest side happier
262+
# add a fake set of microseconds just to make date parsers on the ingest side happier
264263
date = date_dt.strftime("%Y-%m-%dT%H:%M:%S") + ".000000Z"
265264
except (ValueError, OverflowError):
266265
logging.warning(

services/apps/mailing_list_integration/src/test/test_noteren.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import pytest
2222

23+
from crowdmail.errors import CommandExecutionError
2324
from crowdmail.services.parse import noteren
2425

2526
HERE = os.path.dirname(os.path.abspath(__file__))
@@ -247,20 +248,20 @@ def fake_git_run(git_dir, args, stdin=None):
247248
assert calls[1][1][0] == "cat-file"
248249

249250

250-
def test_get_email_from_git_missing_commit_exits(monkeypatch):
251+
def test_get_email_from_git_missing_commit_raises(monkeypatch):
251252
monkeypatch.setattr(noteren, "git_run_command", lambda *args, **kwargs: (0, b"", b""))
252-
with pytest.raises(SystemExit):
253+
with pytest.raises(CommandExecutionError):
253254
noteren.get_email_from_git("/tmp/repo.git", "deadbeef")
254255

255256

256-
def test_get_email_from_git_missing_blob_exits(monkeypatch):
257+
def test_get_email_from_git_missing_blob_raises(monkeypatch):
257258
def fake_git_run(git_dir, args, stdin=None):
258259
if args[0] == "ls-tree":
259260
return 0, b"blob123\n", b""
260261
return 1, b"", b"missing"
261262

262263
monkeypatch.setattr(noteren, "git_run_command", fake_git_run)
263-
with pytest.raises(SystemExit):
264+
with pytest.raises(CommandExecutionError):
264265
noteren.get_email_from_git("/tmp/repo.git", "deadbeef")
265266

266267

0 commit comments

Comments
 (0)