Improve DNS address book management #269
Conversation
claddyy
force-pushed
the
mang_addr
branch
2 times, most recently
from
e0c14ba
to
0206c5a
Compare
- write addresses to files for each POST request from Maker - introduce different types for directory server errors. - nit: fix typo in `addres_rpc`
There was a problem hiding this comment.
Concept ACK.
Remove the error handling code. All of those are done in #268, and this PR will create a lot of conflict with it. Just keep the address related changes.
Need some more changes as suggested below.
One valuable thing to check in this PR is to test the address book persistence. A simple test case would look as below.
- Create a static address file with some hardcoded address value. You can put the file in the tests folder as
tests/data/dns/address.dat. - Init the DirectoryServer with the correct datadir path such that
address.datis loaded. - Assert that after loading the all the new address are present in
directory.adddresseshashset. - Update the address book with one more new address. You can send a vanilla TCP POST request to the Directory server port.
- Stop the directory server. Start the server again.
- Assert that all the old and new addresses are present in the loaded structure.
This would ensure that the expected address book behavior is maintained by the directory server.
src/bin/directoryd.rs
Outdated
| @@ -27,5 +27,5 @@ fn main() { | |||
|
|
|||
| let directory = Arc::new(DirectoryServer::new(args.data_directory, Some(conn_type)).unwrap()); | |||
|
|
|||
| start_directory_server(directory); | |||
| start_directory_server(directory).expect("Error in starting directory server."); | |||
There was a problem hiding this comment.
clippy doesn't let me commit without removing it. I can resolve the merge conflicts, while merging the PR. but I believe I won't have the permissions for that?
So, shall I use --no-verify flag and compile the code? But this might make other things vulnerable.
As of now, I've addressed a few of your comments in bb7d3be
src/market/directory.rs
Outdated
| if address_file.exists() { | ||
| let file = File::open(&address_file)?; | ||
| let reader = BufReader::new(file); | ||
| for address in reader.lines().map_while(Result::ok) { | ||
| addresses | ||
| .write() | ||
| .map_err(|_| DirectoryServerError::LockError)? | ||
| .insert(address); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
| if address_file.exists() { | |
| let file = File::open(&address_file)?; | |
| let reader = BufReader::new(file); | |
| for address in reader.lines().map_while(Result::ok) { | |
| addresses | |
| .write() | |
| .map_err(|_| DirectoryServerError::LockError)? | |
| .insert(address); | |
| } | |
| } | |
| if let Ok(file) = File::open(&address_file) { | |
| let reader = BufReader::new(file); | |
| let lines: HashSet<String> = reader.lines().filter_map(Result::ok).collect(); | |
| addresses.write().unwrap().extend(lines); | |
| } |
More more better way to do the task.
| @@ -111,6 +132,7 @@ impl DirectoryServer { | |||
| connection_type_value, | |||
| ) | |||
| .unwrap_or(connection_type_value), | |||
| addresses, | |||
There was a problem hiding this comment.
| addresses, | |
| addresses, | |
| ..Default::default() | |
| }) |
directly use default values for shutdown , rpc_port instead of explicitly mentioning them.
src/market/directory.rs
Outdated
| handle_client(&mut stream, address_arc); | ||
| .map_err(DirectoryServerError::Io)?; | ||
| if let Err(e) = handle_client(&mut stream, &directory) { | ||
| log::error!("Error handling client: {:?}", e); |
There was a problem hiding this comment.
quick nit:
| log::error!("Error handling client: {:?}", e); | |
| log::error!("Error handling client request: {:?}", e); |
closes #256