-
Notifications
You must be signed in to change notification settings - Fork 564
feat(core): Replace fixed parameters with environment variables #3075
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 enhances configuration flexibility by replacing the hard-coded MAX_RETRY_DELAY_MS constant with an environment variable-based configuration. The change allows operators to customize the maximum retry delay for controller requests without modifying code.
Key Changes:
- Replaced fixed 10-second max retry delay with
AUTOMQ_MAX_RETRY_DELAY_MSenvironment variable - Added import for
com.automq.stream.utils.Systemsutility class - Maintained backward compatibility with 10-second default value
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
core/src/main/scala/kafka/log/stream/s3/network/ControllerRequestSender.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
core/src/main/scala/kafka/log/stream/s3/network/ControllerRequestSender.java
Outdated
Show resolved
Hide resolved
| private static final Logger LOGGER = LoggerFactory.getLogger(ControllerRequestSender.class); | ||
|
|
||
| private static final long MAX_RETRY_DELAY_MS = 10 * 1000; // 10s | ||
| private static final long MAX_RETRY_DELAY_MS = Systems.getEnvLong("AUTOMQ_MAX_RETRY_DELAY_MS", 10L * 1000); // 10s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the env name could be more specific such as AUTOMQ_CONTROLLER_REQUEST_MAX_RETRY_DELAY_MS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been adjusted. Please review it again. Thank you.
… (#3078) Co-authored-by: yx9o <yangx_soft@163.com>
Replace
kafka.log.stream.s3.network.ControllerRequestSender#MAX_RETRY_DELAY_MSwith a value that can be set via environment variables.