langchaingo icon indicating copy to clipboard operation
langchaingo copied to clipboard
verified publisher icon tmc

fix: Improve bedrock provider extraction from model id

Open lucaronca opened this issue 11 months ago • 2 comments

Currently, the bedrock implementation assumed that the model ID would start with the provider name, which meant it can't handle cases like passing the ARN as the model ID or where model ID uses a different format. For instance, AWS customer support recommends using a custom-formatted model ID to work around a bedrock bug (see: AWS Repost). I have now updated the code to extract the provider using a more flexible approach that supports these custom formats.

PR Checklist

  • [x] Read the Contributing documentation.
  • [x] Read the Code of conduct documentation.
  • [x] Name your Pull Request title clearly, concisely, and prefixed with the name of the primarily affected package you changed according to Good commit messages (such as memory: add interfaces for X, Y or util: add whizzbang helpers).
  • [x] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • [x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. Fixes #123).
  • [x] Describes the source of new concepts.
  • [x] References existing implementations as appropriate.
  • [x] Contains test coverage for new functions.
  • [x] Passes all golangci-lint checks.

lucaronca avatar Feb 06 '25 17:02 lucaronca

Would it be possible to get a review on this? I would like to see it in. I am facing the same issue with langchain go. Thank you!

karsharma10 avatar Apr 24 '25 23:04 karsharma10

Thanks for sharing the workaround for inference profile ARNs. Would like to see this in as well. 😄

sugkrish avatar Apr 25 '25 15:04 sugkrish

@tmc - sorry for the ping, but is there an opportunity to get this reviewed/merged? It seems like a fairly safe change, and would greatly help those of us using bedrock 😄

RicePatrick avatar May 05 '25 15:05 RicePatrick

@tmc We also have this fix for bedrock which is quite important, let me know if it is ok. Thank you!

lucaronca avatar May 13 '25 11:05 lucaronca

@tmc We also have this fix for bedrock which is quite important, let me know if it is ok. Thank you!

Yep sorry about the delay, LGTM and merged.

tmc avatar May 13 '25 11:05 tmc

Do you know when is expected to generate a new version that contains this change?

cHiv0rz avatar Jul 24 '25 17:07 cHiv0rz

@tmc - any chance of a new version cut soon so that we would be able to use this? :pray:

RicePatrick avatar Aug 13 '25 04:08 RicePatrick