Implement the store_filestore_part action#184
Conversation
| mod tests { | ||
|
|
||
| use super::*; | ||
|
|
There was a problem hiding this comment.
What is the expected behavior if the agent is asked to store a part of a file that is already complete?
I expect that it should be idempotent, i.e. also return Ok(Complete), but we should have a test verifying that behavior as well.
There was a problem hiding this comment.
It was idempotent but it was actually more an accident that a conscious decision. After thinking about this, I created #185 that ensures we fail to store the same part twice.
I prefer not to add too much tests in the action itself as it just delegates to the filestore and any edge cases should be resolved and tested there.
I extended the comments on the proto fields to document the behaviour.
There was a problem hiding this comment.
Oh, I misread your comment!
store a part of a file that is already complete
This is another edge case I haven't considered! I will update #185 to also fail in that case.
| // are ready. | ||
| // | ||
| // All parts belonging to the same file are expected to have the same digest. | ||
| // In case of discrepancies, arbitrary value will be used. |
There was a problem hiding this comment.
This line is not very clear. I presume it means that if multiple parts have different digests, there is no guarantee as to which one will be picked.
But also, can't we check for that in the action and make that an error?
There was a problem hiding this comment.
Reworded.
But also, can't we check for that in the action and make that an error?
Nope. We verify the file only after the file is complete, so "the last part" wins. We don't know what hashes previous action invocations provided.
2 participants
No description provided.