-
-
Notifications
You must be signed in to change notification settings - Fork 10
Refactor services for improved async handling and error resilience #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @TimilsinaBimal, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant architectural improvements aimed at bolstering the application's performance, reliability, and user experience. It focuses on enhancing asynchronous operations, making external API interactions more resilient to transient failures, and refining the core recommendation logic to deliver more diverse and relevant content suggestions. Additionally, a new feature for displaying dynamic announcements has been added to the user interface. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request significantly refactors several services to improve asynchronous handling and error resilience. Key enhancements include the introduction of retry mechanisms with exponential backoff for external API calls, the use of semaphores to prevent rate-limiting, and ensuring exceptions in background tasks are properly logged. The recommendation engine has also been substantially upgraded with more sophisticated scoring logic, a diversification step using Maximal Marginal Relevance (MMR), and more robust data processing. These changes greatly improve the application's performance, stability, and the quality of recommendations. I've identified a few areas for minor improvements to further enhance correctness and performance.
| final_items.append(item) | ||
|
|
||
| return final_items[:limit] | ||
| return final_items |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function no longer slices the result to limit. The caller in catalogs.py doesn't seem to apply any limit either, which could lead to returning more items than intended by the API contract. It's better to enforce the limit within this service to ensure consistent behavior.
| return final_items | |
| return final_items[:limit] |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silently passing on all exceptions with except Exception: pass can hide important issues, such as problems with the TMDB API key or network connectivity. It's better to log these exceptions, at least at a debug level, to aid in troubleshooting.
| except Exception: | |
| pass | |
| except Exception as e: | |
| logger.debug(f"Failed to detect media type as movie for tmdb:{tmdb_id}: {e}") |
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def _mmr_select(cands: list[dict], k: int, lamb: float = 0.75) -> list[dict]: | ||
| selected: list[dict] = [] | ||
| remaining = cands[:] | ||
| while remaining and len(selected) < k: | ||
| if not selected: | ||
| best = remaining.pop(0) | ||
| selected.append(best) | ||
| continue | ||
| best_item = None | ||
| best_score = float("-inf") | ||
| for cand in remaining[:50]: # evaluate a window for speed | ||
| rel = cand.get("_final_score", 0.0) | ||
| div = 0.0 | ||
| for s in selected: | ||
| div = max(div, _candidate_similarity(cand, s)) | ||
| mmr = lamb * rel - (1 - lamb) * div | ||
| if mmr > best_score: | ||
| best_score = mmr | ||
| best_item = cand | ||
| if best_item is None: | ||
| break | ||
| selected.append(best_item) | ||
| try: | ||
| remaining.remove(best_item) | ||
| except ValueError: | ||
| pass | ||
| return selected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation of _mmr_select has some performance concerns. Operations like remaining.pop(0) and remaining.remove(best_item) on a list are O(N), which can be inefficient for larger candidate sets.
Consider these improvements:
- Use
collections.dequefor theremainingcandidates to make popping from the left an O(1) operation. - Instead of
remove(), consider tracking indices. You could maintain a list ofremaining_indicesand remove from that, or use a boolean array to mark items as selected, which is more efficient.
* feat: add option to rename, disable, reorder catalogs * feat: new recommendation system based on weights * feat: dynamic row generator * feat: add dynamic row generator with country flag * feat: add watched.loved row * feat: new ui with stremio auth button * feat: add languages selection button in ui for tmdb meta * feat: add option to enable rpdb api key for posters * feat: ui improvements * chore: add vercel app env * chore: improve recommendation weights and refactor * chore: remove unnecessary country flags * add workflows to bump version * chore: bump version to 1.0.0-rc.1 * chore: bump version to 1.0.0-rc.2 * chore: use redis token from config * fix: priotrize keywords for row generation than genre * chore: bump version to v1.0.0-rc.3 * feat: add option to delete account * refactor: update token deletion API to use DELETE / and implement deep cloning for catalog data in the frontend, removing conditional name handling. * opt: improve recommendations using larger candidate pool and better similarity methods * feat: Add genre exclusion UI, store excluded genres in user settings,and apply them during catalog generation (#21) * feat: Add genre exclusion UI, store excluded genres in user settings, and apply them during catalog generation. * refactor: simplify filtering recommendations by excluded genres using list comprehension * refactor: streamline genre exclusion logic for similarity recommendations * feat: Rework UI into a multi-step setup wizard with account management and section navigation (#22) * feat: Rework UI into a multi-step setup wizard with Watchly account management and section navigation * critical: check if user already exists * refactor: remove disabled class from source code button * feat: add translation service to translate catalog names to user language (#23) * feat: add option to rename, enable disable catalogs (#24) * feat: Implement user ID-based token management and Stremio user info fetching, replacing credential-derived tokens and direct login. (#25) * refactor: code refactoring and remove legacy code and components (#26) * feat: add option to rename, enable disable catalogs * feat: add option to rename, enable disable catalogs * feat: Implement user ID-based token management and Stremio user info fetching, replacing credential-derived tokens and direct login. * feat: Implement user ID-based token management and Stremio user info fetching, replacing credential-derived tokens and direct login. * feat: Refactor token management to use a dedicated token model, remove token salt, and streamline catalog refresh logic. * feat: Encrypt auth keys in token store and enforce 401 for invalid tokens across endpoints. * feat: Introduce token redaction utility and apply it to log messages for enhanced security. * feat: Introduce token redaction utility and apply it to log messages for enhanced security. * fernet client management * feat: add migration script to migrate old tokens to new system * feat: check and only update tokens that are installed from the same system * opt: give more weight to items watched recently * docs: add contributing guidelines and acknowledgements * opt: optimize translation service * chore: bump version to v1.0.0-rc.6 * feat: add gemini service to generate catalog names (#29) * Refactor services for improved async handling and error resilience (#30) * refactor: fetch loved and liked items at once and use that * Refactor services for improved async handling and error resilience * chore: bump version to v1.0.0 * feat: better bare row name generation (#32)
No description provided.