feat(cloudflare-client): export CLOUDFLARE_LOGPUSH_FIELDS (LLMO-5869)#1796
feat(cloudflare-client): export CLOUDFLARE_LOGPUSH_FIELDS (LLMO-5869)#1796IrisAlexandrescu wants to merge 5 commits into
Conversation
Adds requestLogpushOwnership, listLogpushJobs, and createLogpushJob to CloudflareClient — the zone-scoped Cloudflare Logpush API calls needed to automate BYOCDN Cloudflare log forwarding, replacing the manual dashboard setup flow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lpers (LLMO-5869) Single source of truth for the byocdn-cloudflare Logpush field list, so the Logpush job payload built in spacecat-api-service can source output_options.field_names from here instead of duplicating the literal array. prettifyLogForwardingConfig now references the same exported constant rather than an inline duplicate.
…t test (LLMO-5869) CI caught this: index.test.js enumerates every named export from src/index.js and fails on drift. Missed updating it when CLOUDFLARE_LOGPUSH_FIELDS was added.
|
This PR will trigger a minor release when merged. |
There was a problem hiding this comment.
Hey @IrisAlexandrescu,
⚠ Degraded review - no spec document was found for this change (searched the PR links, the touched repos' docs, the architecture/guidelines docs, and linked Jira). This review covers code-level quality but could not validate the change against an agreed design, so confidence is reduced. Add a spec link (PR template section 4) and re-request review for a full-confidence pass.
Verdict: Approve - clean, well-scoped addition following established patterns.
Complexity: MEDIUM - medium diff across two packages.
Changes: Adds three Cloudflare Logpush API methods to CloudflareClient and extracts a shared CLOUDFLARE_LOGPUSH_FIELDS constant into spacecat-shared-utils (7 files).
Non-blocking (1): minor issues and suggestions
- nit:
CLOUDFLARE_LOGPUSH_FIELDSis a plain mutable array; any consumer can accidentally.push()and corrupt the shared reference -packages/spacecat-shared-utils/src/cdn-helpers.js:49. Considerexport const CLOUDFLARE_LOGPUSH_FIELDS = Object.freeze([...])to communicate immutability intent at zero runtime cost.
Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 39s | Cost: $5.01 | Commit: 68cfb5bc81ad317f12982e60c38f93187f58198c
If this code review was useful, please react with 👍. Otherwise, react with 👎.
MysticatBot
added
ai-reviewed
…S (review feedback)
Review feedback on this PR:
- Freeze CLOUDFLARE_LOGPUSH_FIELDS (Object.freeze) so consumers can't accidentally
mutate the shared array reference (nit23uec, MysticatBot).
- Add getZone(zoneId) -> GET /zones/{zone_id}, mirroring listRoutes' single-param
shape. Needed by spacecat-api-service's Logpush controller (LLMO-5869, separate
PR) to verify a caller-supplied zoneId's registrable domain actually matches the
site it's being onboarded for, before creating a Logpush job for it.
…feature/LLMO-5869-cloudflare-logpush
3 participants
Summary
requestLogpushOwnership,listLogpushJobs,createLogpushJob) were already added toCloudflareClientin a prior commit on this branch.CLOUDFLARE_LOGPUSH_FIELDS, exported fromcdn-helpers.js, as the single source of truth for the 9byocdn-cloudflareLogpush field names — needed by thespacecat-api-servicecontroller (separate PR) to build a real Logpush job'soutput_options.field_nameswithout duplicating the literal array.prettifyLogForwardingConfig's ownbyocdn-cloudflareformatter now references this same constant instead of an inline duplicate, so there is exactly one place this list lives.Related Issues
LLMO-5869
Test plan
packages/spacecat-shared-utils— 46 tests passing, 100% statements/lines, 96.55% branch (pre-existing gap, unrelated to this change)npx eslintclean on changed filesDownstream
spacecat-api-service's Cloudflare Logpush controller (LLMO-5869, separate draft PR) depends on this being merged + published (semantic-release) before its dependency bump on@adobe/spacecat-shared-utilsand@adobe/spacecat-shared-cloudflare-clientcan land for real.