feat(data-warehouse): implement the aws batch import source - #110791
Conversation
|
😎 Merged successfully - details. |
|
Risk: No findings The only change since the last review is an added unit test in test_aws_batch.py covering the AwsBatchSource adapter's delegation of credential validation and resumable-manager creation. It is test-only code with no runtime effect; the production source module it exercises is unchanged. Sentinel reviewed |
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🤖 CI report
|
| First copy | Second copy | Lines | Tokens |
|---|---|---|---|
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/dynamodb/source.py:66 |
40 | 141 |
products/warehouse_sources/backend/temporal/data_imports/sources/acculynx/source.py:18 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:23 |
14 | 137 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cost_anomaly_detection/source.py:145 |
31 | 111 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_organizations/source.py:151 |
31 | 111 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_budgets/source.py:140 |
26 | 94 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_cost_explorer/source.py:137 |
23 | 80 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/source.py:109 |
products/warehouse_sources/backend/temporal/data_imports/sources/aws_ses/source.py:150 |
22 | 76 |
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
✅ Backend coverage — all changed backend lines covered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ████████████████████ 100.0% (449 / 449)
All changed backend lines are covered ✅
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
demo |
███████████░░░░░░░░░ 53.5% |
1,447 / 2,707 |
batch_exports |
████████████████░░░░ 81.3% |
21,575 / 26,552 |
cdp |
██████████████████░░ 88.3% |
4,559 / 5,164 |
mcp_analytics |
██████████████████░░ 89.2% |
5,038 / 5,651 |
product_tours |
██████████████████░░ 89.3% |
1,340 / 1,500 |
dashboards |
██████████████████░░ 89.6% |
6,924 / 7,727 |
notebooks |
██████████████████░░ 90.2% |
15,304 / 16,971 |
signals |
██████████████████░░ 90.4% |
60,160 / 66,529 |
cohorts |
██████████████████░░ 90.5% |
8,534 / 9,434 |
data_warehouse |
██████████████████░░ 90.6% |
14,375 / 15,860 |
streamlit_apps |
██████████████████░░ 90.8% |
2,684 / 2,956 |
managed_warehouse |
██████████████████░░ 91.0% |
10,252 / 11,263 |
data_modeling |
██████████████████░░ 91.2% |
10,562 / 11,584 |
tasks |
██████████████████░░ 91.3% |
79,542 / 87,115 |
exports |
██████████████████░░ 91.7% |
9,684 / 10,566 |
business_knowledge |
██████████████████░░ 92.0% |
8,472 / 9,208 |
engineering_analytics |
██████████████████░░ 92.2% |
11,497 / 12,475 |
today |
██████████████████░░ 92.3% |
999 / 1,082 |
early_access_features |
███████████████████░ 92.6% |
1,339 / 1,446 |
conversations |
███████████████████░ 92.6% |
29,225 / 31,559 |
stamphog |
███████████████████░ 92.8% |
8,109 / 8,742 |
canvas |
███████████████████░ 92.9% |
7,155 / 7,703 |
approvals |
███████████████████░ 93.0% |
3,974 / 4,271 |
mcp_registry |
███████████████████░ 93.1% |
1,670 / 1,794 |
notifications |
███████████████████░ 93.2% |
1,144 / 1,228 |
error_tracking |
███████████████████░ 93.3% |
16,389 / 17,573 |
surveys |
███████████████████░ 93.4% |
6,644 / 7,113 |
autoresearch |
███████████████████░ 93.4% |
8,837 / 9,457 |
slack_app |
███████████████████░ 93.7% |
14,611 / 15,600 |
context_layer |
███████████████████░ 93.8% |
3,415 / 3,639 |
web_analytics |
███████████████████░ 93.9% |
23,680 / 25,229 |
billing_alerts |
███████████████████░ 94.1% |
2,094 / 2,226 |
mcp_store |
███████████████████░ 94.3% |
8,959 / 9,501 |
alerts |
███████████████████░ 94.7% |
9,319 / 9,844 |
wizard |
███████████████████░ 94.7% |
6,150 / 6,496 |
ai_observability |
███████████████████░ 94.7% |
25,860 / 27,307 |
workflows |
███████████████████░ 94.7% |
15,187 / 16,034 |
reminders |
███████████████████░ 94.8% |
760 / 802 |
review_hog |
███████████████████░ 95.0% |
11,750 / 12,362 |
annotations |
███████████████████░ 95.1% |
817 / 859 |
endpoints |
███████████████████░ 95.1% |
9,234 / 9,706 |
customer_analytics |
███████████████████░ 95.2% |
26,116 / 27,428 |
marketing_analytics |
███████████████████░ 95.3% |
19,450 / 20,413 |
posthog_ai |
███████████████████░ 95.4% |
2,530 / 2,653 |
actions |
███████████████████░ 95.5% |
756 / 792 |
logs |
███████████████████░ 95.5% |
15,468 / 16,200 |
experiments |
███████████████████░ 95.5% |
33,000 / 34,548 |
data_catalog |
███████████████████░ 95.6% |
4,402 / 4,606 |
tracing |
███████████████████░ 95.6% |
3,536 / 3,699 |
replay_vision |
███████████████████░ 95.6% |
29,371 / 30,708 |
growth |
███████████████████░ 95.7% |
11,381 / 11,888 |
skills |
███████████████████░ 95.8% |
6,972 / 7,274 |
messaging |
███████████████████░ 95.9% |
3,834 / 3,999 |
product_analytics |
███████████████████░ 96.0% |
28,521 / 29,696 |
revenue_analytics |
███████████████████░ 96.4% |
1,889 / 1,959 |
user_interviews |
███████████████████░ 96.5% |
2,870 / 2,974 |
feature_flags |
███████████████████░ 96.6% |
27,119 / 28,060 |
access_control |
███████████████████░ 96.7% |
7,739 / 8,007 |
warehouse_sources |
███████████████████░ 97.3% |
471,349 / 484,399 |
data_quality |
████████████████████ 97.5% |
7,701 / 7,895 |
metrics |
████████████████████ 98.0% |
4,252 / 4,338 |
analytics_platform |
████████████████████ 98.3% |
2,784 / 2,833 |
pulse |
████████████████████ 98.5% |
2,046 / 2,078 |
live_debugger |
████████████████████ 99.2% |
626 / 631 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (8)📝 WalkthroughWalkthroughThe change adds AWS Batch as an implemented HTTP source. It configures four resource endpoints, signs REST JSON requests with SigV4, handles pagination and resumable state, and validates credentials and API versions. It also adds source descriptions and tests for request signing, retrieval, retries, error handling, and validation. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to The jobs table will omit submitted, pending, succeeded, and failed jobs. Enumerate and resume across statuses before merging to avoid an incomplete dataset. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The integration uses fixed AWS operations, signed requests, credential redaction, and the existing checkpoint lifecycle. No authorization bypass or privilege escalation was established. Remaining uncertainty concerns overlapping retries, configuration changes during recovery, and deployed network controls. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Usage-based review receipt
Note This review exceeded your plan’s limits and used usage-based reviews—free during trial. After your trial, your Enterprise plan’s existing billing terms apply. Manage usage-based reviews. Comment |
A new stamphog review started for this PR — the fresh verdict replaces this approval.
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Approved.
Self-contained new warehouse source written by an owning-team author with STRONG familiarity, following the existing aws_organizations SigV4 pattern. Region is validated before any credentials are sent, secrets are redacted, and the tests are thorough. No guard suppressions were added.
- Author wrote 88% of the modified lines and has 102 merged PRs in these paths (familiarity STRONG).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 551L, 5F substantive, 976L/7F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (976L, 7F, single-area, feat) |
| stamphog 2.3.1 | .stamphog/policy.yml @ fb0a442 · reviewed head fb0a442 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Enumerate jobStatus values for the jobs table. · aws_batch.py:167-215
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/aws_batch.py:167-215
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftEnumerate
jobStatusvalues for the jobs table.
AWS_BATCH_ENDPOINTS["jobs"]binds this request toListJobsfor API version2016-08-10. AWS returns onlyRUNNINGjobs whenjobStatusis omitted. TheJOB_NAMEwildcard does not change this default, so the table can omit all non-running jobs.Iterate through all supported statuses and persist the status index with
nextTokenso pagination and resume cover every status.Suggested fix
+_JOB_STATUSES = ("SUBMITTED", "PENDING", "RUNNABLE", "STARTING", "RUNNING", "SUCCEEDED", "FAILED") + + @frozen class AwsBatchResumeConfig: next_token: str | None = None queue_arns: list[str] | None = None queue_index: int = 0 + job_status_index: int = 0 complete: bool = False @@ payload["jobQueue"] = queues[state.queue_index] - # A name wildcard includes every status without seven separate status walks. + payload["jobStatus"] = _JOB_STATUSES[state.job_status_index] payload["filters"] = [{"name": "JOB_NAME", "values": ["*"]}] @@ - queue_index = state.queue_index + (token is None) - state = replace(state, next_token=token, queue_index=queue_index, complete=queue_index >= len(queues)) + if token is not None: + state = replace(state, next_token=token) + elif state.job_status_index + 1 < len(_JOB_STATUSES): + state = replace(state, next_token=None, job_status_index=state.job_status_index + 1) + else: + queue_index = state.queue_index + 1 + state = replace( + state, + next_token=None, + job_status_index=0, + queue_index=queue_index, + complete=queue_index >= len(queues), + )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 690603cf-4f7b-4cde-aca1-a677d1b28cf0
📒 Files selected for processing (1)
products/warehouse_sources/backend/temporal/data_imports/sources/aws_batch/tests/test_aws_batch.py
Limit details: You’ve used all 12 included reviews currently available.
|
Checked the latest CodeRabbit concern against the AWS Batch 🦉 via talyn.dev |
|
/trunk merge |
Problem
Changes
jobsjob_queuescompute_environmentsjob_definitionsaws_organizationssource.unreleasedSourceremoved.SOURCES.mdrow.How did you test this code?
Test rationale: the source is new, so no existing test covers its transport.
Release status
Automatic notifications
Docs update