Fixes #15456: Handle ttl parameter correctly in shared-files API #2384
Conversation
|
Commit modified removed warnings |
|
Commit modified |
1 similar comment
|
Commit modified |
| .expect("Unable to write file"); | ||
| fs::write( | ||
| format!("shared-files/{}.metadata", peek), | ||
| format!("{}expires={}\n", metadata_string, timestamp.unwrap()), |
There was a problem hiding this comment.
we shouldn't unwrap here, we need to properly manage the error
There was a problem hiding this comment.
I just saw it is actually correct as the error cases returns early. Could you change the definition of timestamp to something like
let timestamp = match parse_ttl(..) {
Ok(ttl) => ttls,
Err(_) => return StatusCode::from_u16(500).unwrap(),
}to make it clearer?
|
Commit modified |
| @@ -97,7 +97,7 @@ impl RawNodesList { | |||
| } | |||
|
|
|||
| pub struct NodesList { | |||
| list: RawNodesList, | |||
| pub list: RawNodesList, | |||
There was a problem hiding this comment.
It should not be public, but you can add a get_... method in impl
|
Commit modified |
| @@ -123,6 +123,10 @@ impl NodesList { | |||
| Ok(NodesList { list: nodes, my_id }) | |||
| } | |||
|
|
|||
| pub fn get_nodeslist(&self) -> &NodesList { | |||
There was a problem hiding this comment.
The idea of the fields not being public is to prevent code from outside of this module to rely on the internal representation of the nodes list, which may evolve. Instead you should create a dedicated method for what you are trying to to. In this case, it is even easier is it seems to already be there :)
|
Commit modified |
|
OK, merging this PR |
https://issues.rudder.io/issues/15456