Skip to content

Conversation

@andrest50
Copy link
Contributor

This pull request updates the chart version for the lfx-v2-indexer-service. The only change is a minor version bump in the Helm chart metadata.

  • Incremented the version field from 0.4.3 to 0.4.4 in charts/lfx-v2-indexer-service/Chart.yaml to reflect a new release.

Signed-off-by: Andres Tobon <[email protected]>
@andrest50 andrest50 requested a review from a team as a code owner September 25, 2025 22:16
Copilot AI review requested due to automatic review settings September 25, 2025 22:16
@coderabbitai
Copy link

coderabbitai bot commented Sep 25, 2025

Walkthrough

Bumped Helm chart version and adjusted how the OpenSearch scroll timeout is formatted in a migration script (changed to an integer-minute string).

Changes

Cohort / File(s) Summary
Helm chart version bump
charts/lfx-v2-indexer-service/Chart.yaml
Update version field: 0.4.30.4.4.
Migration script: scroll timeout formatting
scripts/migration/001_add_access_query_fields/main.go
Change ScrollRequest scroll value from scrollTimeout.String() to fmt.Sprintf("%dm", int(scrollTimeout.Minutes())) (integer minutes representation).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly summarizes the Helm chart version bump to 0.4.4, which is a primary change in this pull request, even though it does not mention the additional migration script alteration.
Description Check ✅ Passed The description accurately details the Helm chart version bump from 0.4.3 to 0.4.4, which aligns with part of the changeset and therefore remains on topic.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch andrest50/bump-chart

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

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 updates the Helm chart version for the lfx-v2-indexer-service from 0.4.3 to 0.4.4, representing a minor version bump for a new release.

  • Incremented chart version to reflect new release

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

The scroll timeout needs to be in minutes format (e.g., "5m") rather than
Go's duration string format. This fixes the scroll bad request error.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

Signed-off-by: Andres Tobon <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d8f9067 and 5f5a1c2.

📒 Files selected for processing (1)
  • scripts/migration/001_add_access_query_fields/main.go (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: MegaLinter

Comment on lines 248 to 250
"scroll_id": scrollID,
"scroll": scrollTimeout.String(),
"scroll": fmt.Sprintf("%dm", int(scrollTimeout.Minutes())),
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Revert truncating the scroll timeout to whole minutes

Casting to int(scrollTimeout.Minutes()) floors anything below 60 seconds to 0m and drops the seconds component for values like 90s → 1m, changing the effective timeout and potentially invalidating the scroll context. Keep the original duration string so fractional minutes remain intact.

-		"scroll":    fmt.Sprintf("%dm", int(scrollTimeout.Minutes())),
+		"scroll":    scrollTimeout.String(),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"scroll_id": scrollID,
"scroll": scrollTimeout.String(),
"scroll": fmt.Sprintf("%dm", int(scrollTimeout.Minutes())),
}
"scroll_id": scrollID,
"scroll": scrollTimeout.String(),
}
🤖 Prompt for AI Agents
In scripts/migration/001_add_access_query_fields/main.go around lines 248 to
250, the code currently truncates the scroll timeout to whole minutes via
int(scrollTimeout.Minutes()), which floors durations under a minute to "0m" and
drops seconds; change the payload to use the original duration string (e.g.,
scrollTimeout.String()) or otherwise format the duration to include seconds so
fractional minutes/seconds are preserved when sending the scroll timeout.

@andrest50 andrest50 merged commit cf4e6f5 into main Sep 25, 2025
5 checks passed
@andrest50 andrest50 deleted the andrest50/bump-chart branch September 25, 2025 22:57
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