feat: register_bedrock now supports arn #118
Merged
Conversation
This file contains 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
abhisheksoamj
approved these changes
Feb 1, 2025
| import httpx | ||
|
|
||
| import re |
There was a problem hiding this comment.
PEP convention for import
javelin_sdk/client.py
Outdated
|
|
||
| if arn: | ||
| # Retrieve the inference profile using the ARN. | ||
| get_profile_response = self.bedrock_client.get_inference_profile( |
There was a problem hiding this comment.
We could save these responses or mapping in javelin-client, ARN aren't going to change once they are created in AWS ecosystem. What do you think?
javelin_sdk/client.py
Outdated
|
|
||
| # Update the request URL to use the Javelin endpoint. | ||
| # Replace the scheme and netloc based on self.base_url while preserving path and query. | ||
| new_netloc = urlparse(self.base_url).netloc |
There was a problem hiding this comment.
I suspect changing inference profile ARN with model id could break their inference profile output... like
Miss cost optimization
Lack monitoring and metrics or whatever application inference has to offer.
what do you say?
…d. Fail silently in case of error (this needs to be enhanced to add async tracing). 2. added logic to pass a default_route_name for bedrock models optionally when registering bedrock_client, 3. added logic to always etract the model and set the x-javlein-model header (needed for model_spec in case if are not rewriting the url and we get an arn instead of model id
…of hardcoding 'https'
ankumar
approved these changes
Feb 3, 2025
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.
This branch has the following changes: