[REFAC#391] PriorityResolver chain → publisher 측 이동 + 모든 PublishX 통과 - #409
Conversation
…391) 메타 #385 Sub 6 (마지막) — fetcher/worker 의 PriorityResolver chain 을 publisher 측으로 이동하고, 모든 PublishX 메소드가 buildMessage 를 통해 resolver chain 을 일관 통과하도록 통합. ## 이동 - internal/processor/fetcher/worker/resolver.go → internal/publisher/resolver.go - 패키지 선언만 변경, 인터페이스 / 구현 로직 동일 (ExplicitPriorityResolver, SourcePriorityResolver, RuleBasedPriorityResolver, DefaultPriorityResolver, CompositeResolver, ChainablePriorityResolver) - PriorityResolver 인터페이스는 publisher.go 에서 이미 정의 — 중복 제거 ## 모든 PublishX 통과 - publisher.buildMessage 가 p.resolver.Resolve(job) 를 호출하도록 변경 — 단일 진입점 - PublishChained / PublishSeed / PublishJob / KafkaImmediateRetryScheduler.Enqueue / RedisDelayedRetryScheduler.republish 모두 buildMessage 경유 → 자동으로 resolver 통과 - chain.go 의 명시적 resolver.Resolve(job) 호출 제거 (이중 평가 회피) ## 명시 priority 보존 - main.go wiring 에 ExplicitPriorityResolver 를 chain 1순위 로 추가 - 발행자 (scheduler seed entry / worker retry / fetcher upgrader 등) 가 job.Priority 를 사전 명시한 경우 그 값이 보존됨 — ExplicitPriorityResolver.CanResolve 가 true 반환 - 후속 Source/Rule 기반 resolver 는 explicit 미설정 시에만 적용 ## fetcher/worker.PoolManager - PriorityResolver 필드 타입 → publisher.PriorityResolver (구 worker.PriorityResolver 정의 제거) - Publish() 의 명시 priority 갱신 (job.Priority = priority) 제거 — publisher 가 buildMessage 안에서 처리. resolver 가 stateless · idempotent 라 manager 의 로깅용 평가는 안전 ## 검증 - go build ./internal/... ./cmd/... ./test/... ./examples/... — pass - go test -race -count=1 -timeout=180s ./test/... — 전 패키지 통과 - gofmt clean 본 sub 머지로 메타 #385 (Publisher 통합) 완료. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR moves crawler job priority resolution logic from the worker package into the publisher package, implementing a chain-of-responsibility resolver pattern. All ChangesPriority Resolver Chain Migration to Publisher
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/processor/fetcher/worker/manager.go`:
- Around line 168-172: PoolManager.Publish currently calls
m.resolver.Resolve(job) unconditionally which panics when m.resolver is nil;
change Publish to guard that call by checking m.resolver != nil before invoking
Resolve and only assign priority when the resolver exists, otherwise leave
priority nil/zero so Publisher.buildMessage (which accepts a nil resolver) can
run the fallback path; update references in Publish to use the guarded priority
variable and ensure no other code paths assume Resolve was called.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65b8d120-45b6-4f56-886b-397c9c00010d
📒 Files selected for processing (5)
cmd/issuetracker/main.gointernal/processor/fetcher/worker/manager.gointernal/publisher/chain.gointernal/publisher/publisher.gointernal/publisher/resolver.go
…t Major) CodeRabbit 피드백: - publisher.buildMessage 가 nil resolver 를 fail-safe 로 허용하지만, manager.Publish 의 로깅용 m.resolver.Resolve(job) 호출은 무조건적 → 테스트 wiring (nil resolver) 에서 panic. - nil resolver 시 job.Priority 를 그대로 사용 — publisher 의 fail-safe 정책과 일관. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request centralizes priority resolution logic within the publisher package, moving it from the worker package to ensure a single source of truth for all publishing paths. The PriorityResolver chain is now integrated into the buildMessage helper, and an ExplicitPriorityResolver has been added to preserve pre-defined job priorities. Feedback from the review highlights a redundant priority calculation in the PoolManager's Publish method used for logging and points out potential side effects caused by buildMessage modifying the Priority field of the CrawlJob pointer, suggesting improved documentation or API refinement to clarify this behavior.
…edium) gemini 피드백: - publisher.buildMessage 가 외부 주입된 *job 의 Priority 를 직접 수정 → 호출자가 원본 보존을 기대할 경우 의도치 않은 부작용. PublishChained / PublishSeed 같은 내부 생성 job 은 문제 없으나 PublishJob 으로 외부 job 을 발행할 때 surprise side effect. - local 복사본 (j := *job) 의 Priority 만 갱신하고 Marshal / Topic / Headers 모두 j 기준 으로 구성. CrawlJob 은 작은 struct 이라 복사 비용 무시 가능. - manager.Publish 의 로깅용 m.resolver.Resolve(job) 평가는 job 미수정이라 publisher 의 side-effect-free buildMessage 와 일관 — 양쪽 결과 동일 (resolver idempotent). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
연관 이슈
Closes #391
부모 메타: #385 — Publisher 통합 Sub 6 (마지막)
본 sub 머지로 메타 #385 (Publisher 통합) 완료.
구현 내용
1. resolver.go 위치 이동
ChainablePriorityResolver/ExplicitPriorityResolver/SourcePriorityResolver/RuleBasedPriorityResolver/DefaultPriorityResolver/CompositeResolver모두 이동PriorityResolver인터페이스는 publisher.go 에 이미 정의 — 중복 제거2. 모든 PublishX 가 resolver chain 통과
publisher.buildMessage가p.resolver.Resolve(job)를 호출하도록 변경 — 단일 진입점.chain.go 의 명시적
job.Priority = p.resolver.Resolve(job)호출 제거 (이중 평가 회피).3. ExplicitPriorityResolver 를 chain 1순위 로
발행자가
job.Priority를 사전 명시한 경우 (scheduler seed entry / worker retry / fetcher upgrader 등) 그 값이 보존됨 —ExplicitPriorityResolver.CanResolve가 true 반환.4. fetcher/worker.PoolManager 정리
PriorityResolver필드 타입 →publisher.PriorityResolver(구 worker.PriorityResolver 정의 제거)Publish()의job.Priority = priority갱신 제거 — publisher 가 buildMessage 안에서 처리. resolver 가 stateless · idempotent 라 manager 의 로깅용 평가는 안전.CI / 머지 게이트 점검
gofmt -l internal/ cmd/ test/ examples/— cleango build ./internal/... ./cmd/... ./test/... ./examples/...— passgo test -race -count=1 -timeout=180s ./test/...— 전 패키지 통과[REFAC#391][REFAC]:prefix + 한국어변경 영향 범위 + 위험도
메타 #385 완료
본 sub 머지 후 메타 #385 (Publisher 통합) 의 6 sub 모두 완료:
본 PR 머지 시 메타 #385 도 함께 close.
롤백 계획
PR revert 시 resolver 파일이 worker 측으로 다시 이동 + main.go wiring 원복 — 5 파일 동시 원복으로 회귀 없음.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores