Hi, Virajith Jalaparti
Thanks a lot for update the patch and address the comments. And appreciate much for taking our comments into consideration.
The other way to deal with this would be to add setDir to ReplicaInfo but to avoid File as a parameter.
What I am worried about is that, this constraint might not hold true for wider audience. E.g., If you implement recovery logic in Azure or S3, the input should not be a File based replica.
it should take in an URI. Which do you think is better?
Maybe using StorageLocation as input? There was some efforts to abstract File to StorageLocation.
FinalizedReplica in the current type hierarchy, is a LocalReplica.
Would that still be the case for Azure? I have saw a few cases that it is not a file-based solution. Also, FinalizedReplica should be the most common class in the ReplicaInfo hierarchy, so I guess many vendors might expect using FinalizedReplica to directly represent the block data.
The way I was thinking about it was that the state of ReplicaInfo should be known using ReplicaInfo::getState(), and not using the type system.
I don't have strong option against the solution proposed here. The concerns of getting rid of type system and using
to indicate the class type might make the other developer difficult to have good confident to write correct code, because like dynamic language as python/ruby, the bugs become hard to spot during compile time. If we can come up a good test coverage, the concern can be overcome to some extend.