-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add user
param to on_init
command section
#68
Conversation
Signed-off-by: Plakhotnikov Vladimir <v.plahotnikov@yclients.tech>
WalkthroughThe update involves restructuring how configuration is managed in a Go application. A new Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (3)
- config.go (1 hunks)
- init.go (1 hunks)
- plugin.go (1 hunks)
Additional comments: 5
config.go (1)
- 38-39: Adding the
User
field to theInitConfig
struct is consistent with the PR's objective to allow specifying a user for theon_init
command. Ensure that the new field is properly documented and that its usage is clear to the users of the library.init.go (3)
16-17: The refactoring to include an
InitConfig
object in thecommand
struct is a good approach to simplify parameter passing and improve maintainability.33-37: The check for a non-empty
User
field and the subsequent call toutils.ExecuteFromUser
is a critical addition. Ensure that theExecuteFromUser
function is robust and handles potential errors when switching users.40-40: The use of a timer to enforce the
ExecTimeout
is appropriate. Ensure that the timer is stopped in all code paths to avoid resource leaks.plugin.go (1)
- 162-162: The simplification in the
Serve
method to pass the entireOnInit
struct is a good practice as it reduces the complexity of the function signature and makes future changes easier.
user
param to on_init
commanduser
param to on_init
command section
} | ||
|
||
func newCommand(log *zap.Logger, env map[string]string, cmd []string, execTimeout time.Duration) *command { | ||
func newCommand(log *zap.Logger, cfg *InitConfig) *command { |
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.
Good catch with *InitConfig
👍
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.
LGTM, thanks @Kaspiman 👍
Reason for This PR
closes: roadrunner-server/roadrunner#1585
Description of Changes
Added abillity to configure
user
foron_init
command section.License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]
git commit -s
).CHANGELOG.md
.Summary by CodeRabbit
New Features
Refactor