design: add client.md#1573
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1573 +/- ##
=======================================
Coverage 48.58% 48.58%
=======================================
Files 28 28
Lines 4250 4250
=======================================
Hits 2065 2065
Misses 1750 1750
Partials 435 435Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I think its weird to add documents like this for only one item. Either do them all or none at all. It just creates weird documents that are narrow and confuse the readers.
There was a problem hiding this comment.
Is there other item?
Using listing instead of table might be better?
There was a problem hiding this comment.
Want to add a TODO here with more information about how containerd is a client-heavy design?
There was a problem hiding this comment.
Are we assuming the reader should know the pros and cons of gRPC and how it lends itself to implementing 3rd-party containerd clients? It may be helpful to provide an example.
There was a problem hiding this comment.
We've already made the choice. ;)
@estesp has a talk somewhere where he discusses the decision. There are also some old discussions about how avoiding http specification time speeds up implementation.
A link to grpc.io would be good.
There was a problem hiding this comment.
are push&pull interactions the only thing a 3rd-party client would need to reimplement?
There was a problem hiding this comment.
No, there is a huge list. We should probably document that list here.
e18dec6 to
3d38a44
Compare
|
Updated PR, including GC labels |
There was a problem hiding this comment.
These aren't restricted to these types, are they?
There was a problem hiding this comment.
@dmcgowan PTAL (why do we need timestamp here?)
There was a problem hiding this comment.
@AkihiroSuda The timestamp tells you when the label expires, letting you reserve objects before they are referenced elsewhere.
There was a problem hiding this comment.
Right now I am using the creation time. While garbage collection is done by the serve, a "prune" operation will be client side and these timestamps can be used to hint any client doing pruning as to which objects may be old. It is a client decision but by adding a timestamp here it gives clients more information it may need. This value is not read by the server, that should be noted, it is only for client use.
There was a problem hiding this comment.
Also allow gc.ref.content.${something} to allow adding multiple links. It would be inaccurate to say "Digest string of children blobs." since each label may only refer to a single digest. To refer to multiple children multiple label keys must be used using a client define string after gc.ref.content..
3d38a44 to
bfdd5ca
Compare
|
updated PR |
Signed-off-by: Akihiro Suda <[email protected]>
bfdd5ca to
0689e63
Compare
|
@dmcgowan PTAL? |
|
LGTM I still think this doc needs a much large expansion, but that might be on me. |
|
Same for me. Its a weird document. |
|
I'd feel more comfortable with this as part of the package doc, maybe. |
|
I'm just going to close this PR. It's not adding any real value to the reader and adds more confusion in turn. It's just random information with no real consistency for a design document and is not up to par with the other design documents that are in depth. |
|
Could you open an alternate PR? |
|
@AkihiroSuda I'd suggest adding this to the godoc, where it makes sense to document details of working with the Go client. |
update cni config version
Signed-off-by: Akihiro Suda [email protected]