Sync: fix implementation and document #101
Conversation
| SyncStatus = | ||
| Name | ||
| StatusCode | ||
|
|
||
| RepoCommandParam = |
There was a problem hiding this comment.
The insert topic only accepts RepoCommandParam that contains ObjParam.
The sync/join topic only accepts RepoCommandParam that contains SyncParam.
This suggests that these should be two separate types: ObjCommandParam and SyncCommandParam.
There was a problem hiding this comment.
I agree. We really should separate them.
| RepoCommandParam = | ||
| 1 * (OBJECT-PARAM-TYPE TLV-LENGTH ObjectParam) | ||
| 0* (OBJECT-PARAM-TYPE TLV-LENGTH ObjParam) | ||
| 0* (SYNC-PARAM-TYPE TLV-LENGTH SyncParam) | ||
|
|
||
| RepoCommandRes = |
There was a problem hiding this comment.
The insert topic only returns RepoCommandRes that contains ObjStatus.
The sync/join topic only accepts RepoCommandRes that contains SyncStatus.
This suggests that these should be two separate types: ObjCommandRes and SyncCommandRes.
| @@ -5,30 +5,42 @@ Most repo commands and status reports are Data packets whose Content contains | |||
| ``RepoCommandParam`` or ``RepoCommandRes`` structure. | |||
| These Data are issued via Pub-Sub protocol. | |||
| Each ``RepoCommandParam`` and ``RepoCommandRes`` contains | |||
| multiple ``ObjectParam`` and ``ObjectResult``, resp. | |||
| multiple ``ObjParam`` and ``ObjStatus``, resp. | |||
| These structures are defined as follows: | |||
|
|
|||
There was a problem hiding this comment.
Please define how to handle TLV-TYPE evolvability in this protocol.
There was a problem hiding this comment.
I don't know the answer but it seems not supported now.
@tianyuan129 Do we want to design a better protocol? The current version looks messy.
There was a problem hiding this comment.
it seems not supported now
You could declare "all TLV-TYPE numbers are critical".
In this case, any future addition would be a breaking change.
There was a problem hiding this comment.
I don't know the answer but it seems not supported now. @tianyuan129 Do we want to design a better protocol? The current version looks messy.
I kinda agree. The current Sync implementation is just quick code, time to revisit. But when you say a better protocol, are you referring to Sync itself or the entire repo command framework? The current repo command is very much object storage orientated, it could be hard to harmoniously put Sync function into the protocol.
There was a problem hiding this comment.
I mean the repo command framework. I think it needs a revisit to leave spaces for Sync and probably my Linked Object proposal.
| Name | ||
| StatusCode | ||
| [InsertNum] | ||
| [DeleteNum] | ||
|
|
||
| SyncStatus = |
There was a problem hiding this comment.
The use of SyncStatus shall be mentioned in sync/join and sync/leave pages.
There was a problem hiding this comment.
Well, actually it is not used. Though the server returns this struct, the client discards it.
| @@ -41,6 +53,12 @@ These structures are defined as follows: | |||
|
|
|||
| RegisterPrefix = REGISTER-PREFIX-TYPE TLV-LENGTH Name | |||
|
|
|||
| SyncPrefix = SYNC-PREFIX-TYPE TLV-LENGTH Name | |||
|
|
|||
| DataNameDedupe = SYNC-DATA-NAME-DEDUPE-TYPE TLV-LENGTH ; TLV-LENGTH = 0 | |||
There was a problem hiding this comment.
Just a reminder this is pure hack for NDN Workspace. Ideally we should let apps define naming conventions.
Fix: #100