Skip to content

Conversation

tiulpin
Copy link
Member

@tiulpin tiulpin commented Jul 17, 2025

  • includes changes from zarechneva/enable-bedrock-tests
  • fixes integration tests and provides proper auth settings for bedrock integration

@tiulpin tiulpin force-pushed the tv/koog-bedrock-here-we-go-again branch 2 times, most recently from 670c7b2 to 5358ec4 Compare July 17, 2025 15:39
@tiulpin tiulpin requested a review from aozherelyeva July 17, 2025 15:44
@tiulpin tiulpin marked this pull request as ready for review July 17, 2025 15:45
@tiulpin tiulpin force-pushed the tv/koog-bedrock-here-we-go-again branch from 5358ec4 to 842274c Compare July 18, 2025 11:01
@tiulpin tiulpin requested a review from Copilot July 18, 2025 11:03
@tiulpin tiulpin changed the title Support AWS_SESSION_TOKEN in BedrockClient Support AWS_SESSION_TOKEN in BedrockClient Jul 18, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for AWS session tokens in the BedrockClient to enable the use of temporary AWS credentials. The changes primarily focus on enabling Bedrock integration tests that were previously disabled.

  • Adds awsSessionToken parameter to BedrockLLMClient constructors and related factory functions
  • Updates integration tests to use proper AWS environment variables and enables previously disabled Bedrock tests
  • Adds a smoke test to verify Bedrock credentials before running full integration tests

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
SimplePromptExecutors.jvm.kt Adds awsSessionToken parameter to simpleBedrockExecutor function
BedrockLLMClient.kt Updates constructor to accept and use awsSessionToken for authentication
TestUtils.kt Fixes AWS environment variable name and adds session token utility function
SingleLLMPromptExecutorIntegrationTest.kt Enables Bedrock tests and updates to use session token
BedrockCredentialsSmokeTest.kt New smoke test to verify Bedrock credentials
env.template.properties Adds AWS credential environment variables template
build.gradle.kts Adds AWS SDK dependencies for Bedrock tests
libs.versions.toml Adds bedrock and sts SDK library definitions
heavy-tests.yml Updates workflow to run Bedrock smoke test
Comments suppressed due to low confidence (1)

integration-tests/src/jvmTest/kotlin/ai/koog/integration/tests/utils/TestUtils.kt:42

  • The warning message is misleading. When AWS_SESSION_TOKEN is not set, null is returned, not a 'default session token'. Consider changing to 'using null (no session token)' or removing the message entirely since returning null for optional session tokens is expected behavior.
                println("WARNING: environment variable `AWS_SESSION_TOKEN` is not set, using default session token")

@tiulpin tiulpin force-pushed the tv/koog-bedrock-here-we-go-again branch 3 times, most recently from d83fcaa to 44c92b7 Compare July 18, 2025 14:12
Copy link

github-actions bot commented Jul 18, 2025

Qodana for JVM

368 new problems were found

Inspection name Severity Problems
Check Kotlin and Java source code coverage 🔶 Warning 349
Unused import directive 🔶 Warning 15
Vulnerable imported dependency 🔶 Warning 4
@@ Code coverage @@
+ 64% total lines covered
7978 lines analyzed, 5180 lines covered
# Calculated according to the filters of your coverage tool

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

@aozherelyeva aozherelyeva force-pushed the tv/koog-bedrock-here-we-go-again branch from 44c92b7 to e7b3731 Compare July 22, 2025 13:29
@aozherelyeva aozherelyeva force-pushed the tv/koog-bedrock-here-we-go-again branch from 5895a3f to 18c3dbd Compare July 23, 2025 13:42
@tiulpin tiulpin merged commit 958aa0c into develop Jul 28, 2025
6 checks passed
@tiulpin tiulpin deleted the tv/koog-bedrock-here-we-go-again branch July 28, 2025 14:40
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.

2 participants