Implement Evaluator Versions #402
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Introduced a new `fireworks_client.py` module to centralize Fireworks SDK client creation. - Updated CLI and evaluation modules to use the new `create_fireworks_client` function instead of direct instantiation of the Fireworks class. - Enhanced handling of API key, account ID, base URL, and extra headers through environment variables. - Added tests for the new Fireworks client factory to ensure proper functionality and configuration.
- Added functionality to load environment variables from .env.dev or .env as a fallback when the auth module is imported. - Updated the API key verification process to allow explicit base URL handling, defaulting to dev.api.fireworks.ai if not provided. - Removed redundant environment variable loading code from platform_api module.
- Introduced functionality to create evaluator versions using parameters such as commit hash, entry point, and requirements. - Updated the upload endpoint call to utilize the newly created evaluator version ID instead of a hardcoded test version ID. - Added error handling for missing evaluator version ID in the response to ensure robustness during code uploads.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
update to latest once SDK is published with changes
- Implemented a try-except block to handle APIStatusError during evaluator creation. - Added logic to check for existing evaluators and retrieve the existing one if a conflict occurs (status code 409). - Enhanced logging for better traceability of evaluator creation process.
…d signature introspection, avoiding unnecessary API requests during help invocations.
…dating polling functions to target specific evaluator versions. Refactor related CLI commands and tests to accommodate these changes, ensuring clearer status messages and improved error handling.
| logger.warning(f"Code upload failed (evaluator created but code not uploaded): {upload_error}") | ||
| # Don't fail - evaluator is created, just code upload failed | ||
| # Return None for version_id since upload failed | ||
| return result, None |
There was a problem hiding this comment.
Tar file not cleaned up when upload fails
Medium Severity
When the upload process fails after the tar.gz file is created at tar_path, the exception handler returns (result, None) without removing the tar file. The cleanup at lines 347-349 is only reached on the success path. This leaves a {dir_name}.tar.gz file in the user's working directory after every failed upload attempt, which could be large and confusing.
Additional Locations (1)
…iable loading into local test command. Introduced functions to find and retrieve values from .env files, enhancing configuration management for Docker tests.
…iable loading into local test command. Introduced functions to find and retrieve values from .env files, enhancing configuration management for Docker tests.
…mplement-evaluator-versions
…kspace so it should include the .env file
- improving environment variable management and preventing conflicts with other .env files.
- improving environment variable management and preventing conflicts with other .env files.
…valuator-versions
- Added `upload_and_ensure_evaluator` function to handle evaluator uploads and ensure the latest version is ACTIVE. - Updated `create_evj_command` and `create_rft_command` to utilize the new upload function. - Removed redundant polling logic from `create_rft.py` and `create_evj.py`, centralizing it in the new utility function. - Adjusted tests to mock the new upload function correctly.
- Implemented functions to check for existing secrets and confirm overrides before uploading to Fireworks. - Enhanced user interaction with double confirmation for overriding existing secrets, including fallback for non-interactive environments. - Updated the upload command to handle new and existing secrets separately, ensuring proper management during uploads.
- Added `handle_secrets_upload` function to `create_evj.py`, `create_rft.py`, and `upload.py` for managing secrets with double verification for existing entries. - Streamlined the upload process by consolidating secret management logic, enhancing user interaction during uploads. - Removed redundant secret loading functions from `upload.py` to improve code clarity and maintainability.
…e code clarity and maintainability.
- Replaced manual parsing of .env files with `dotenv_values()` for improved handling of comments, quotes, and multi-line values. - Updated `load_secrets_from_env_file` function to return a filtered dictionary of environment variables, enhancing code clarity and maintainability.
|
|
||
| rft_parser.add_argument("--yes", "-y", action="store_true", help="Non-interactive mode") | ||
| rft_parser.add_argument("--dry-run", action="store_true", help="Print planned SDK call without sending") | ||
| rft_parser.add_argument("--force", action="store_true", help="Overwrite existing evaluator with the same ID") |
There was a problem hiding this comment.
Missing --env-file argument for rft and evj commands
Medium Severity
Both create_rft_command and create_evj_command access env_file via getattr(args, "env_file", None) and pass it to handle_secrets_upload, but neither rft_parser nor evj_parser defines the --env-file CLI argument. Users cannot specify a custom env file path for these commands, and the code always receives None. The upload command correctly defines this argument but it's missing from the create subcommands.
Additional Locations (2)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
Implements versioned evaluator workflow and streamlines platform integration.
evaluator_versions.create,get_upload_endpoint,validate_upload;create_evaluationnow returns(result, version_id)create_fireworks_client()withFIREWORKS_EXTRA_HEADERSsupport; migrates SDK calls acrossevaluation, RFT, platform secrets, and dataset uploadep create evjcommand to create Evaluation Jobs with auto arg generation and local validation hookscli_commands/secrets.py) with selection and double-confirm overrides; integrated intoupload,create rft, andcreate evjfireworks-aito1.0.0a22; adds comprehensive tests for secrets, client factory, evaluator versioning, CLI flows; VS Code launch example added and.gitignoreadjustedWritten by Cursor Bugbot for commit 37f4856. This will update automatically on new commits. Configure here.