feat: ✨ improve image fetching speeds#171
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🧰 Additional context used📓 Path-based instructions (2)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/!(ltsm)/**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR extends the LINE OBS client with configurable download options and dedicated HTTP client routing, then instruments image handling to measure operation timings and log structured metrics. New OBS download methods accept TID and OBSPop parameters with improved context-aware retry logic; upload operations are routed through a dedicated OBS HTTP client; and image conversion now tracks and reports download, decryption, and upload durations. ChangesOBS Download Options and Image Instrumentation
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/line/client.go (1)
727-752:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle exhausted 202/404 retries before the generic non-200 branch.
Line 727only retries whileattempt < obsMaxRetries. On the last 202/404, execution falls through toLine 737and returns the generic HTTP error instead, soLine 752is unreachable and callers never get the intended “still processing after N retries” failure.Suggested fix
- if (resp.StatusCode == 202 || resp.StatusCode == 404) && attempt < obsMaxRetries { - resp.Body.Close() - select { - case <-time.After(obsRetryDelay): - case <-ctx.Done(): - return nil, ctx.Err() - } - continue - } + if resp.StatusCode == 202 || resp.StatusCode == 404 { + resp.Body.Close() + if attempt == obsMaxRetries { + return nil, fmt.Errorf("OBS download failed: media still processing after %d retries", obsMaxRetries) + } + select { + case <-time.After(obsRetryDelay): + case <-ctx.Done(): + return nil, ctx.Err() + } + continue + } @@ - return nil, fmt.Errorf("OBS download failed: media still processing after %d retries", obsMaxRetries) + return nil, fmt.Errorf("OBS download failed: media still processing after %d retries", obsMaxRetries)🤖 Prompt for 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. In `@pkg/line/client.go` around lines 727 - 752, The 202/404 handling currently only retries when attempt < obsMaxRetries, but on the final 202/404 it falls through into the generic non-200 branch so the "still processing after N retries" error is never returned; update the response-status handling block (the branch that checks resp.StatusCode == 202 || resp.StatusCode == 404) to explicitly handle the exhausted-retries case: close resp.Body and return an error like "OBS download failed: media still processing after %d retries" when attempt >= obsMaxRetries, otherwise sleep on obsRetryDelay and continue retrying; keep use of obsMaxRetries, obsRetryDelay, ctx, attempt and the surrounding OBS download logic intact.
🤖 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 `@pkg/connector/handlers/image.go`:
- Around line 41-48: The current log call dumps the entire data.ContentMetadata
map (Interface("content_metadata", data.ContentMetadata)) which exposes vendor
metadata; remove that Interface field and instead extract and log only the
specific metadata keys you need (e.g., media type, filename, size or other
derived fields) by safely reading from data.ContentMetadata (check types and
existence) and add them as individual Str/Int/Bool fields in the same
h.Log.Debug() chain; update the logging sequence in the handler where the
Debug() is built (the chain that includes oid, data.ID, downloadOptions.TID,
etc.) to include only those explicit metadata fields or omit metadata entirely.
---
Outside diff comments:
In `@pkg/line/client.go`:
- Around line 727-752: The 202/404 handling currently only retries when attempt
< obsMaxRetries, but on the final 202/404 it falls through into the generic
non-200 branch so the "still processing after N retries" error is never
returned; update the response-status handling block (the branch that checks
resp.StatusCode == 202 || resp.StatusCode == 404) to explicitly handle the
exhausted-retries case: close resp.Body and return an error like "OBS download
failed: media still processing after %d retries" when attempt >= obsMaxRetries,
otherwise sleep on obsRetryDelay and continue retrying; keep use of
obsMaxRetries, obsRetryDelay, ctx, attempt and the surrounding OBS download
logic intact.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d75fbacd-b9d0-419a-a3ab-872f13de69ef
📒 Files selected for processing (2)
pkg/connector/handlers/image.gopkg/line/client.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-docker
- GitHub Check: Lint with 1.25
- GitHub Check: build-docker
- GitHub Check: Lint with 1.25
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/connector/handlers/image.gopkg/line/client.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/connector/handlers/image.gopkg/line/client.go
Fixed some issues from indent
1 participant
No description provided.