Skip to content

fix: handle model check for finetune/batches #1020

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

Merged
merged 2 commits into from
Apr 11, 2025

Conversation

b4s36t4
Copy link
Contributor

@b4s36t4 b4s36t4 commented Apr 1, 2025

Code Quality bug detected type: bug fix

Author Description

🔄 What Changed

  • Added a new constant BEDROCK_NO_MODEL_ENDPOINTS that lists all endpoints that don't require model validation
  • Modified the model validation logic to skip the check for endpoints in the exemption list
  • Prevents unnecessary 'Model is required' errors for finetune and batch-related operations

🔍 Impact of the Change

  • Improves user experience by removing unnecessary validation errors
  • Makes the API more intuitive by not requiring model parameters for endpoints that don't need them
  • Maintains backward compatibility with existing code

📁 Total Files Changed

  • 1 file modified (src/providers/bedrock/api.ts) with 12 additions and 1 deletion

🧪 Test Added

  • N/A

🔒 Security Vulnerabilities

  • N/A

Title:

  • fix: handle model check for finetune/batches

Description:

🔄 What Changed

  • Added a new constant BEDROCK_NO_MODEL_ENDPOINTS that lists all endpoints that don't require model validation (listFinetunes, retrieveFinetune, cancelFinetune, listBatches, retrieveBatch, getBatchOutput, cancelBatch).
  • Modified the model validation logic in src/providers/bedrock/api.ts to skip the check for endpoints present in the BEDROCK_NO_MODEL_ENDPOINTS list.

🔍 Impact of the Change

  • Prevents unnecessary 'Model is required' errors for finetune and batch-related operations.
  • Improves user experience by removing incorrect validation errors.
  • Makes the API more intuitive for relevant endpoints.
  • Maintains backward compatibility.

📁 Total Files Changed

  • 1 file modified (src/providers/bedrock/api.ts) with 12 additions and 1 deletion.

🧪 Test Added

  • N/A

🔒 Security Vulnerabilities

  • N/A

Motivation:

  • Fixes an issue where endpoints that don't require a model parameter were incorrectly throwing validation errors.
  • Improves the developer experience when working with finetune and batch-related operations.

Related Issues:

  • N/A

Quality Recommendations

  1. Consider adding unit tests to verify the model validation logic for both endpoints requiring a model and those included in the BEDROCK_NO_MODEL_ENDPOINTS list. This will prevent regressions in the future.

  2. For improved lookup performance, especially if the list of endpoints might grow, consider using a Set instead of an Array for BEDROCK_NO_MODEL_ENDPOINTS.

@VisargD VisargD merged commit 8edde62 into Portkey-AI:main Apr 11, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants