harden REST reliability pipeline - #48
Conversation
|
@ErrorDoc404 What is the purpose of this PR? What warranted the "hardening"? PS. This looks ENTIRELY generated with an LLM. the code quality is very low compared to the rest of the codebase. Complete disregard for code conventions and clear style being followed across the project. |
|
You're right that I didn't explain the purpose of the PR clearly enough. The term "hardening" was pretty vague. What I'm trying to address is reliability in Lilybird's core runtime, mainly around the two areas that interact with external services: REST requests and the Gateway/WebSocket connection. On the REST side, I wanted a more consistent way of dealing with rate limits and temporary server/network failures. The idea is to retry only where it's safe to do so, while giving callers proper errors for things like rate limits and network failures. On the Gateway side, the goal is to make reconnects, resumes, heartbeats, and shard connections more reliable, especially when a connection becomes stale or stops responding. I'm also taking #47 into account since those compiler changes are now part of the direction Lilybird is taking. I'm not trying to pull all of #47 into this PR; I just want the reliability work to fit properly with that architecture instead of adding more coupling to And regarding the code quality — that's fair. Some of the code in the PR doesn't follow the existing Lilybird conventions closely enough. I should have caught that before opening the PR. I'm going back through the affected files and cleaning them up to match the project's existing style rather than considering the implementation finished just because the functionality works. I'll also update the PR description so the problem, reasoning, and scope are much clearer. |
Hi, is this Codex? or... what can I call you? nice to make your acquaintance. The current REST helpers already give you proper errors. You said you wanted to take #47 in to account, but you simply added conflicts against it by "not trying to pull all of" it in. As to the main point of the PR, in making connections reliable, it's not a concern for the library; its a concern of the user to implement rate limiting. While thoughtful, its out of scope and unwarranted. I'd love to hear your thoughts on this though, or if you just want to waste a few tokens chatting, thats fine too :) |
|
Rate Limit handling being part of the core is not out of scope just not a priority (see #41), however the state of the PR makes it almost impossible to review. The changes made to the WebSocket manager make little to no sense to me, they are in no way related to the REST wrappers and the PR seems to remove a lot of its intended errors/warnings. You have mentioned sharding but I'm afraid whatever program you are using to help write this PR has mislead you, lilybird does not support any form of sharding at its core, and it is intended to give proper errors when users attempt to "force" such features, at least until #42 gets addressed. My recommendation would be to rebase on #47's branch, and focus only on the rate limiting side of things, other features can be added with extra PRs, but quality must be kept and conventions must be followed, PRs who fail to meet the standards of the repository will not be merged regardless if a human or an AI wrote it. |
No description provided.