main to dev sync #72
main to dev sync #72
Conversation
* feat: enhance CI workflow with Docker Buildx and QEMU setup; add yamlfmt to pre-commit hooks; specify platform in docker-compose * chore: update yamlfmt to v0.11.0; remove platform specification from docker-compose
|
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 represents a substantial architectural refactor aimed at improving the modularity, resilience, and overall performance of the application's core services. Key components like Stremio API interactions, TMDB data fetching, and the recommendation engine have been decoupled into specialized modules. This foundational work enables new features such as persistent Stremio authentication via email/password, more granular control over catalog item limits, and a more sophisticated, on-demand catalog update system. The changes also include significant enhancements to the recommendation algorithm and user interface improvements for a smoother experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and impressive refactoring of the application's core services. The introduction of a BaseClient, the modularization of Stremio interactions into the StremioBundle, and the restructuring of the recommendation logic into a clear pipeline are all excellent changes that will greatly improve maintainability and resilience. The shift to an on-demand catalog update system is also a smart optimization.
My review focuses on a few minor points related to exception handling to make the code even more robust. Overall, this is a fantastic set of changes that modernizes the codebase and adds valuable new features like email/password authentication.
| token = await token_store.store_user_data(user_id, payload_to_store) | ||
| logger.info(f"[{redact_token(token)}] Account {'updated' if existing_data else 'created'} for user {user_id}") |
There was a problem hiding this comment.
The call to token_store.store_user_data is no longer wrapped in a try...except block. The store_user_data function can raise exceptions like redis.exceptions.RedisError or OSError if the Redis store is unavailable. This will result in an unhandled exception and a 500 error for the user. It's important to handle these potential storage errors gracefully, as was done previously.
| token = await token_store.store_user_data(user_id, payload_to_store) | |
| logger.info(f"[{redact_token(token)}] Account {'updated' if existing_data else 'created'} for user {user_id}") | |
| # 5. Store user data | |
| try: | |
| token = await token_store.store_user_data(user_id, payload_to_store) | |
| logger.info(f"[{redact_token(token)}] Account {'updated' if existing_data else 'created'} for user {user_id}") | |
| except RuntimeError as exc: | |
| raise HTTPException(status_code=500, detail="Server configuration error.") from exc | |
| except Exception as exc: | |
| logger.error(f"Token storage failed: {exc}") | |
| raise HTTPException(status_code=503, detail="Storage temporarily unavailable.") from exc |
| try: | ||
| await bundle.auth.get_user_info(auth_key) | ||
| is_valid = True | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
This try...except block catches all exceptions and silently passes, which can hide potential bugs or issues during the auth key validation. It would be better to at least log the exception to aid in debugging. A similar block in app/api/endpoints/manifest.py already includes logging, which would be great to have here for consistency.
except Exception as e:
logger.debug(f"Auth key validation failed during catalog fetch: {e}")
pass| except Exception: | ||
| pass |
There was a problem hiding this comment.
This try...except block for retrieving catalog limits silently ignores all exceptions. This could make it difficult to diagnose problems with user settings, for example if there's a malformed catalog configuration. I recommend adding a log message to capture potential errors.
except Exception as e:
logger.warning(f"Could not parse catalog limits for {cfg_id}: {e}")
pass| except Exception as exc: | ||
| logger.error(f"Token deletion failed: {exc}") | ||
| raise HTTPException(status_code=503, detail="Storage temporarily unavailable.") |
There was a problem hiding this comment.
The exception handling here has been broadened from specific (redis_exceptions.RedisError, OSError) to a generic except Exception. This is generally not recommended as it can catch and mask unexpected errors (like HTTPException which is caught and re-raised just above). It would be more robust to catch the specific exceptions you expect from storage operations. Consider restoring the previous, more specific exception handling. You'll also need to re-import from redis import exceptions as redis_exceptions.
| except Exception as exc: | |
| logger.error(f"Token deletion failed: {exc}") | |
| raise HTTPException(status_code=503, detail="Storage temporarily unavailable.") | |
| except (redis_exceptions.RedisError, OSError) as exc: | |
| logger.error(f"Token deletion failed: {exc}") | |
| raise HTTPException(status_code=503, detail="Storage temporarily unavailable.") from exc |
No description provided.