Bedrock integration backup by vaquarkhan · Pull Request #677 · apache/burr

vaquarkhan · 2026-03-16T06:13:15Z

[Short description explaining the high-level reason for the pull request]

Changes

How I tested this

Notes

Checklist

PR has an informative and human-readable title (this will be pulled into the release notes)
Changes are limited to a single goal (no scope creep)
Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
Any change in functionality is tested
New functions are documented (with a description, list of inputs, and expected output)
Placeholder code is flagged / future TODOs are captured in comments
Project documentation has been updated if adding/changing functionality.

andreahlert

Two features in one PR, would've been easier to review split. Bedrock side is straightforward, the SQS consumer needs work.

andreahlert · 2026-03-24T16:58:21Z

+                                    if s3_key and s3_key.endswith(".jsonl"):
+                                        await self._handle_s3_event(s3_key, event_time)
+
+                                await sqs_client.delete_message(


nit: s3_key = None and event_time = None are dead. They're assigned here but never read, the actual values come from s3_keys_with_times on line 855.

andreahlert · 2026-03-24T16:58:21Z

+
    async def indexing_jobs(
        self, offset: int = 0, limit: int = 100, filter_empty: bool = True
    ) -> Sequence[schema.IndexingJob]:


blocker: if _handle_s3_event raises on the Nth record of a multi-record SQS message, records 0..N-1 get re-indexed on retry with no dedup. Make _handle_s3_event idempotent (check if s3_path exists in LogFile before insert).

andreahlert · 2026-03-24T16:58:21Z

+        self._name = name
+        self._region = region
+        self._guardrail_id = guardrail_id
+        self._guardrail_version = guardrail_version or "DRAFT"


guardrail_version or "DRAFT" silently defaults to DRAFT when only guardrail_id is set. Is that intentional? Feels risky for prod, someone could set a guardrail ID and not realize they're running against an unpublished draft. Same at line 191.

andreahlert · 2026-03-24T16:58:21Z

+        return result, new_state
+
+
+class BedrockStreamingAction(StreamingAction):


IMHO BedrockStreamingAction is a copy-paste of BedrockAction. __init__, _get_client, reads/writes/name properties are all identical, ~70 lines duplicated. Pull them into a _BedrockBase and let each subclass just implement its execution method.

andreahlert · 2026-03-24T16:58:21Z

+        self._region = region
+        self._guardrail_id = guardrail_id
+        self._guardrail_version = guardrail_version or "DRAFT"
+        self._inference_config = inference_config or {"maxTokens": 4096}


just a small nit: inference_config or {"maxTokens": 4096} means inference_config={} gives you the default because empty dict is falsy. Use if inference_config is not None if you want to allow empty configs.

andreahlert · 2026-03-24T16:58:21Z

      - name: Install dependencies
        run: |
-          python -m pip install -e ".[tests,tracking-client,graphviz]"
+          python -m pip install -e ".[tests,tracking-client,graphviz,tracking-server-s3]"


tracking-server-s3 in the main CI install and apache-burr[bedrock] in [tests] (pyproject.toml:101) means every contributor now pulls boto3, aiobotocore, tortoise-orm, aerich even for unrelated PRs. Keep AWS deps in a separate CI job and test group, like the existing test-persister-dbs pattern.

andreahlert · 2026-03-24T16:58:21Z

+        """
+        if self._tracking_mode != TrackingMode.EVENT_DRIVEN or not self._sqs_queue_url:
+            logger.info("Event consumer not configured, skipping")
+            return


Is the data/{project}/... prefix structure guaranteed for all event sources? WDYT about getting the project name from data_file.prefix (you already parse it via DataFile.from_path one line above) instead of a raw split?

andreahlert · 2026-03-24T16:58:22Z

+                        response = await sqs_client.receive_message(
+                            QueueUrl=self._sqs_queue_url,
+                            MaxNumberOfMessages=10,
+                            WaitTimeSeconds=self._sqs_wait_time_seconds,


Nice, auto-creating the project from the S3 event is a good call for the event-driven flow.

andreahlert · 2026-03-24T16:58:22Z

+  })
+}
+
+resource "aws_sqs_queue_redrive_policy" "main" {


+1 on the DLQ + redrive policy setup, clean separation into modules.

…r stream

vaquarkhan · 2026-04-12T07:35:43Z

PR ready for review , also The documentation GitHub Actions workflow reports “No jobs were run” / failed at startup on commit .
https://github.com/apache/burr/actions/runs/24301015848

Could a committer confirm whether workflow approval is needed for fork PRs, or if there’s an infra issue with that workflow?

vaquarkhan · 2026-04-14T18:59:54Z

@andreahlert @skrawcz plz review bedrock changes we moved out to reduce scope of previous PR

skrawcz

Look good. Can we get an example under /examples using the two actions? (and also fix the pre-commit please)

vaquarkhan force-pushed the bedrock-integration-backup branch from 995a1ba to f593ee3 Compare March 16, 2026 08:28

andreahlert suggested changes Mar 24, 2026

View reviewed changes

skrawcz mentioned this pull request Mar 28, 2026

Issue 664 cloud native aws #666

Merged

7 tasks

andreahlert added kind/feature Net-new functionality area/integrations External integrations (LLMs, frameworks) labels Mar 30, 2026

github-actions bot added area/storage Persisters, state storage area/tracking Telemetry, tracing, OpenTelemetry area/ci Workflows, build, release scripts area/examples Relates to /examples pr/needs-rebase Conflicts with main and removed pr/needs-rebase Conflicts with main labels Apr 11, 2026

Add Bedrock integration, tests, docs, and CI

0499dec

vaquarkhan force-pushed the bedrock-integration-backup branch from 4c66bf7 to 0499dec Compare April 12, 2026 04:48

github-actions bot added the area/website burr.apache.org website label Apr 12, 2026

fix(bedrock): streaming writes parity, multi-block text, list join fo…

bbf1c02

…r stream

style: black-format bedrock integration

b22bbfc

skrawcz reviewed Apr 16, 2026

View reviewed changes

		return result, new_state


		class BedrockStreamingAction(StreamingAction):

Conversation

vaquarkhan commented Mar 16, 2026

Changes

How I tested this

Notes

Checklist

Uh oh!

andreahlert left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vaquarkhan commented Apr 12, 2026

Uh oh!

vaquarkhan commented Apr 14, 2026

Uh oh!

skrawcz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andreahlert left a comment •

edited

Loading

skrawcz left a comment •

edited

Loading