-
Notifications
You must be signed in to change notification settings - Fork 13
Added code to create a file that indicates Maintenance Mode #87
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
base: master
Are you sure you want to change the base?
Conversation
cholcombe973
left a comment
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 looks like service.rs is a new checked in file that is generated. I think it should probably be added to the .gitignore file. Overall this looks fine.
…or removing Maintenance flag file
|
Changed SetMaintenance subcommand to set_maintenance |
|
Added additional code to remove the set maintenance status. |
cholcombe973
left a comment
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.
Looks good to me
sdandam
left a comment
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.
Bynar should also bail out if it finds a lock file aka in maintenance mode.
src/client.rs
Outdated
| trace!("handle_set_maintenance called"); | ||
| helpers::set_maintenance(s)?; | ||
| trace!("handle_set_maintenance finished"); | ||
|
|
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.
nit: extra line can be removed
src/client.rs
Outdated
| trace!("handle_unset_maintenance called"); | ||
| helpers::unset_maintenance(s)?; | ||
| trace!("handle_unset_maintenance finished"); | ||
|
|
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.
nit: extra line can be removed
src/disk_manager.rs
Outdated
| result.set_result(ResultType::OK); | ||
| } | ||
| Err(..) => { | ||
| result.set_result(ResultType::ERR); |
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.
I think an error string should also be set here, so that the caller/client knows the reason why the operation failed
src/disk_manager.rs
Outdated
| result.set_result(ResultType::OK); | ||
| } | ||
| Err(..) => { | ||
| result.set_result(ResultType::ERR); |
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.
error string should be set here too.
|
|
||
| pub fn set_maintenance(s: &mut Socket) -> BynarResult<()>{ | ||
| let mut result = OpResult::new(); | ||
| let file = match File::create("/var/log/setMaintenance.lock") { |
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.
This file location should be read from the config file
| } | ||
| pub fn unset_maintenance(s: &mut Socket) -> BynarResult<()>{ | ||
| let mut result = OpResult::new(); | ||
| let file = match fs::remove_file("/var/log/setMaintenance.lock") { |
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.
This file location should be read from config file
No description provided.