Ingester Replica Count via env variable#4
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kapjain-rh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 5 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded a configurable environment variable Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/env.sh`:
- Around line 7-8: The inline comment above TOPIC_PARTITIONS is incorrect — it
says the default is '6' while the variable is exported as 48; update the comment
to reflect the actual default (change the comment text to 'will use "48" if not
set') or if the intended default is 6, change the export to export
TOPIC_PARTITIONS=6; locate the TOPIC_PARTITIONS export in the env.sh snippet and
make the comment and value consistent.
- Around line 9-10: The INGESTER_REPLICAS environment variable exported in
INGESTER_REPLICAS should be forwarded into the OpenShift template processing
call: update the oc process invocation that builds the Lokistack (the oc process
command in scripts/netobserv.sh) to include -p
INGESTER_REPLICAS=$INGESTER_REPLICAS so the template parameter overrides the
default; ensure the variable name matches the exported INGESTER_REPLICAS and is
quoted/expanded the same way other -p parameters are handled in that oc process
call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d771adc5-9798-494f-92e9-f0b2cddafbc9
📒 Files selected for processing (2)
scripts/env.shscripts/loki/lokistack.yaml
| # will use '6' if not set | ||
| export TOPIC_PARTITIONS=48 |
There was a problem hiding this comment.
Default-value comment is stale.
Line 7 says default is '6', but Line 8 sets TOPIC_PARTITIONS=48. Please align the comment with actual behavior.
Proposed fix
-# will use '6' if not set
+# will use '48' if not set
export TOPIC_PARTITIONS=48📝 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.
| # will use '6' if not set | |
| export TOPIC_PARTITIONS=48 | |
| # will use '48' if not set | |
| export TOPIC_PARTITIONS=48 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/env.sh` around lines 7 - 8, The inline comment above TOPIC_PARTITIONS
is incorrect — it says the default is '6' while the variable is exported as 48;
update the comment to reflect the actual default (change the comment text to
'will use "48" if not set') or if the intended default is 6, change the export
to export TOPIC_PARTITIONS=6; locate the TOPIC_PARTITIONS export in the env.sh
snippet and make the comment and value consistent.
|
/test test-deploy |
| if [[ -z $INGESTER_REPLICAS ]]; then | ||
| echo "====> No ingester replicas config was found - using '2'" | ||
| echo "====> To set config, set INGESTER_REPLICAS variable to desired number" | ||
| export INGESTER_REPLICAS=2 | ||
| fi |
There was a problem hiding this comment.
should we set this only for 1x.demo?
| effect: NoExecute | ||
| ingester: | ||
| replicas: 2 | ||
| replicas: ${INGESTER_REPLICAS} |
There was a problem hiding this comment.
how can we set this config only for 1x.demo size? could we do more templating around this config?
|
/test test-deploy |
|
@kapjain-rh: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
2 participants
Summary by CodeRabbit