dosubot[bot] commented on issue #8838:
URL: 
https://github.com/apache/incubator-devlake/issues/8838#issuecomment-4258872452

   <!-- Answer -->
   Thanks for sharing! I took a look at [the 
PR](https://github.com/apache/incubator-devlake/pull/8839) — the approach looks 
solid. You implemented both fixes we discussed:
   
   1. **`ignoreInvalidTimelineResponse` handler** — replaces 
`ignoreDeletedBuilds` and now also skips empty/non-JSON bodies via 
`api.ErrIgnoreAndContinue`, with the body peek-and-restore via `io.NopCloser` 
so valid responses still flow through to the parser. Clean solution.
   2. **Query-level filter** — excluding `result = 'none'` builds in addition 
to `'failed'`, which avoids even hitting the Timeline API for YAML-broken 
builds in the first place.
   
   Good to see unit tests covering the key scenarios (404, empty body, non-JSON 
body, valid JSON passthrough, and valid empty JSON object). The 
defense-in-depth approach here is nice — the query filter prevents most of the 
bad requests, and the `AfterResponse` handler catches anything that slips 
through.
   
   Looks good to me overall. Hope it gets picked up by the maintainers soon!
   
   
   <!-- Dosu Comment Footer -->
   *To reply, just mention 
[@dosu](https://go.dosu.dev/dosubot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=brand-link).*
   
   ---
   <sup>How did I do? 
[Good](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-great_response#message_id=8ca86785-78f7-4071-9ea8-0954be202ad8&feedback_type=great_response)
 | 
[Irrelevant](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-irrelevant_answer#message_id=8ca86785-78f7-4071-9ea8-0954be202ad8&feedback_type=irrelevant_answer)
 | 
[Incorrect](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-incorrect_sources#message_id=8ca86785-78f7-4071-9ea8-0954be202ad8&feedback_type=incorrect_sources)
 | 
[Verbose](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-too_verbose#message_id=8ca86785-78f7-4071-9ea8-0954be202ad8&feedback_type=too_verbose)
 | [Hallucination](https://
 
app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-hallucination#message_id=8ca86785-78f7-4071-9ea8-0954be202ad8&feedback_type=hallucination)
 | [Report 
🐛](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-bug_report#message_id=8ca86785-78f7-4071-9ea8-0954be202ad8&feedback_type=bug_report)
 | 
[Other](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=feedback-other#message_id=8ca86785-78f7-4071-9ea8-0954be202ad8&feedback_type=other)</sup>
   [![Chat with 
Dosu](https://dosu.dev/dosu-chat-badge.svg)](https://app.dosu.dev/b4e8e847-d479-4541-83a8-d88d83fea5c9/ask?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=chat-badge)&nbsp;[![Open
 in 
Cursor](https://dosu.dev/dosu-in-cursor.svg)](https://cursor.com/link/prompt?text=Thanks%20for%20sharing%21%20I%20took%20a%20look%20at%20%5Bthe%20PR%5D%28https%3A//github.com/apache/incubator-devlake/pull/8839%29%20%E2%80%94%20the%20approach%20looks%20solid.%20You%20implemented%20both%20fixes%20we%20discussed%3A%0A%0A1.%20%2A%2A%60ignoreInvalidTimelineResponse%60%20handler%2A%2A%20%E2%80%94%20replaces%20%60ignoreDeletedBuilds%60%20and%20now%20also%20skips%20empty/non-JSON%20bodies%20via%20%60api.ErrIgnoreAndContinue%60%2C%20with%20the%20body%20peek-and-restore%20via%20%60io.NopCloser%60%20so%20valid%20responses%20still%20flow%20through%20to%20the%20parser.%20Clean%20solution.%0A2.%20%2A%2AQuery-level%20filter%2A%2A%20%E2%80%94%20excluding%20%60result%20%
 
3D%20%27none%27%60%20builds%20in%20addition%20to%20%60%27failed%27%60%2C%20which%20avoids%20even%20hitting%20the%20Timeline%20API%20for%20YAML-broken%20builds%20in%20the%20first%20place.%0A%0AGood%20to%20see%20unit%20tests%20covering%20the%20key%20scenarios%20%28404%2C%20empty%20body%2C%20non-JSON%20body%2C%20valid%20JSON%20passthrough%2C%20and%20valid%20empty%20JSON%20object%29.%20The%20defense-in-depth%20approach%20here%20is%20nice%20%E2%80%94%20the%20query%20filter%20prevents%20most%20of%20the%20bad%20requests%2C%20and%20the%20%60AfterResponse%60%20handler%20catches%20anything%20that%20slips%20through.%0A%0ALooks%20good%20to%20me%20overall.%20Hope%20it%20gets%20picked%20up%20by%20the%20maintainers%20soon%21)&nbsp;[![Join
 
Discord](https://img.shields.io/badge/join-5865F2?logo=discord&logoColor=white&label=)](https://go.dosu.dev/discord-bot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-assistant&utm_content=join-discord)&nbsp;[![Share
 on X](https://img.shields.io/bad
 
ge/X-share-black)](https://twitter.com/intent/tweet?text=%40dosu_ai%20helped%20me%20solve%20this%20issue!&url=https%3A//github.com/apache/incubator-devlake/issues/8838)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to