Bryan/update readme for all #21
Conversation
- Updated Dockerfile to include `--extra=all` in the `uv sync` command for comprehensive dependency resolution. - Expanded Makefile to add a new target for running dependency tests. - Modified `pyproject.toml` to define optional dependencies and ensure the `all` extra includes all individual extras. - Improved README.md and added TESTING.md to document the new testing framework for dependency resolution consistency. - Introduced a new test script and Python test file to validate that all dependency combinations resolve to the same lock file. - Updated FastAPI endpoints to handle request headers more effectively.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Added development dependencies for `pytest` and `ruff` in `pyproject.toml`. - Removed outdated test files `test_dependency_resolution.py` and `TESTING.md`. - Refactored `test_dependency_resolution.py` to improve dependency resolution consistency checks and ensure the 'all' extra includes all non-dev dependencies from individual extras. - Updated `uv.lock` with new package versions and dependencies. This commit enhances the testing framework and ensures better management of optional dependencies.
- Deleted `pytest` and `ruff` from the `dev` section in `pyproject.toml` to streamline dependency management.
- Removed `dev` dependencies (`pytest` and `ruff`) from `uv.lock` and adjusted the `provides-extras` section accordingly. - Updated GitHub Actions workflow to use `--extra all` for dependency installation, ensuring all necessary dependencies are included for testing.
…tructions - Updated usage examples to include the `[all]` extra for `lkr-dev-cli` in installation and command execution instructions. - Ensured consistency in command examples throughout the README for better clarity on using the CLI.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @bwebs, 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 enhances the lkr CLI by introducing optional dependencies, improving documentation, and adding tests to ensure dependency resolution consistency. It also fixes a bug in the user attribute updater and updates the Dockerfile to include all dependencies.
Highlights
- Optional Dependencies: Introduces optional dependencies for the
lkrCLI, allowing users to install only the dependencies they need for specific functionalities like MCP server, embed observability, or user attribute updater. - README Updates: Updates the README to reflect the new optional dependencies and provides instructions on how to install them using
uvandpip. - Dependency Resolution Tests: Adds tests to ensure that dependency combinations resolve to the same lock file, maintaining consistency in the dependency tree.
- User Attribute Updater: Modifies the user attribute updater to retrieve the authorization token from the request headers instead of the request object.
- Dockerfile Update: Updates the Dockerfile to include the
--extra=allflag when syncing dependencies.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the project's dependency management by introducing optional dependency groups in pyproject.toml. This improves modularity and allows for more flexible installations. The Dockerfile, Makefile, and README.md have been appropriately updated to reflect these changes, providing clear instructions and a convenient way to run new dependency consistency tests. The addition of comprehensive tests for dependency resolution is a critical improvement, ensuring the reliability of the new optional dependency structure. A minor refactoring in lkr/tools/classes.py also improves code decoupling and testability.
| def get_request_authorization_for_value(self, headers: list[tuple[str, str]]): | ||
| authorization_token = next( | ||
| (header for header in headers if header[0] == "Authorization"), None | ||
| ) | ||
| if authorization_token: | ||
| self.value = authorization_token | ||
| self.value = authorization_token[1] | ||
| else: | ||
| logger.error("No authorization token found") |
There was a problem hiding this comment.
The change from fastapi.Request to list[tuple[str, str]] for headers in get_request_authorization_for_value is a good refactoring. It improves the modularity and testability of the UserAttributeUpdater class by decoupling it from the FastAPI request object. The logic for extracting the Authorization token is correctly updated to handle the new input type.
| # Extract individual extras (excluding 'all') using grep and sed | ||
| # This looks for lines like "mcp = [" and extracts the extra name | ||
| local extras=$(grep -E '^[[:space:]]*[a-zA-Z0-9_-]+[[:space:]]*=' "$pyproject_file" | \ | ||
| sed -E 's/^[[:space:]]*([a-zA-Z0-9_-]+)[[:space:]]*=.*/\1/' | \ | ||
| grep -v '^all$' | \ | ||
| tr '\n' ' ') |
There was a problem hiding this comment.
The discover_extras function uses grep and sed to parse pyproject.toml. While this works for the current format, it's a fragile parsing method that could break if the TOML structure changes slightly. For robustness, consider using a more dedicated TOML parsing tool or a Python script for this logic, similar to test_dependency_resolution.py.
No description provided.