Skip to content

fix: use IAM roles for bedrock access#578

Draft
nrfulton wants to merge 5 commits intogenerative-computing:mainfrom
nrfulton:fix/bedrock_aws_access_key_id
Draft

fix: use IAM roles for bedrock access#578
nrfulton wants to merge 5 commits intogenerative-computing:mainfrom
nrfulton:fix/bedrock_aws_access_key_id

Conversation

@nrfulton
Copy link
Member

@nrfulton nrfulton commented Mar 5, 2026

Misc PR

Type of PR

  • Bug Fix
  • New Feature
  • Documentation
  • Other

Description

Testing

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code as added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

The PR description has been updated. Please fill out the template for your PR to be reviewed.

@mergify
Copy link

mergify bot commented Mar 5, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert|release)(?:\(.+\))?:

@nrfulton
Copy link
Member Author

nrfulton commented Mar 5, 2026

@jakelorocco you are tagged as a reviewer because I changed core.py to only call post-processing if there are no exceptions. The status quo continues to give us pain.

@nrfulton nrfulton changed the title Fix/bedrock aws access key fix: use IAM roles for bedrock access Mar 5, 2026
@jakelorocco
Copy link
Contributor

I think that sounds good. I will review once the changes are done. We will need to make sure the finally block / telemetry stuff doesn't cause issues with this change.

@jakelorocco
Copy link
Contributor

Looks like there's another draft PR open to more comprehensively fix the post process issue: #580 (comment). I will take a closer look tomorrow.

@leothomas
Copy link

leothomas commented Mar 12, 2026

Hey folks - thank you so much for adding support for the BedRock non-mantle endpoints through the LiteLLM backend! I'm currently working on integrating Mellea with a fork of IBM's FactReasoner. I made a couple additional (local) edits to this PR in order to get it to work for me, which include:

  1. Validating authentication using boto3: this PR just checks for AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY environment variables but users can also be authenticated through named AWS profiles (AWS_PROFILE env var) or something like an EC2 instance or Lambda function would directly have an IAM role attached. So I update it to use boto3 to validate that it can reach the Bedrock service, regardless of how it's authneticated
  2. Added explicit handling of logprobs: when executing queries against a Custom Model Import, with a model that supports logprob output (eg: Llama-X.X-instruct), I'm able to use the model_options parameter in the mfuncs.ainstruct function to request logprobs output alongside the main response. I also had to add logprobs support to LiteLLM, which I've forked and I'll be opening a PR for that
  3. Added a num_retries parameter to the bedrock.create_beckrock_litellm_backend() factory to take advantage of liteLLM's native retry-with-backoff - this is important for Custom Model Imports as they are cycled off of Bedrock's compute infrastructure after a period of un-used and therefore have a bit of a coldstart that needs to be managed.

Let me know if these changes would be valuable to contribue back to the main Mellea repo, I'm happy to either open a PR from my own fork (which would be branched off of this PR), or wait till this PR is merged to open mine, or any other way to contribute the changes that makes sense with your workflow; I'm just trying to avoid forking on top of a fork! (the edits are all in the backend/bedrock.py and backends/litellm.py files)

Once again, thank for getting the LiteLLM Bedrock backend working!

@jakelorocco
Copy link
Contributor

@leothomas, I think all these changes sound good. I will check in with @nrfulton today and see if we can get this base PR merged so that your PRs can be opened. I can't test this myself.

The logprobs is definitely something we've been looking into supporting for a little while (mainly just unifying it across backends). We'd love to see your implementation for litellm. Thank you!

@leothomas
Copy link

Hey there! I pulled @nrfulton 's branch into my own forks and then added the edits mentioned above. Here's the branch if you want to check it out: https://github.com/leothomas/mellea/tree/bedrock_aws_access_key_id

@jakelorocco I'll wait for y'all to merge this PR to open one from my branch!

@leothomas
Copy link

leothomas commented Mar 13, 2026

Also, here's the LiteLLM fork with LogProbs: https://github.com/leothomas/litellm/tree/feature/logprobs

I still have to write tests and all that good stuff before opening a PR against LiteLLM

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