csds: update client resource cache to keep and dump metadata #4217
Conversation
menghanl
force-pushed
the
csds_client_keep_md
branch
2 times, most recently
from
9e0d14b
to
050459b
Compare
menghanl
force-pushed
the
csds_client_keep_md
branch
2 times, most recently
from
3e56155
to
822ff9b
Compare
…data Metadata including timestamp, status, raw message, error details. Before, it only keeps parsed resources. Metadata is not populated yet. Will be populated in the next PR.
menghanl
force-pushed
the
csds_client_keep_md
branch
from
822ff9b
to
d3d81ba
Compare
xds/internal/client/dump.go
Outdated
| } | ||
| } | ||
|
|
||
| func (c *clientImpl) dump(t ResourceType) (version string, _ map[string]UpdateWithMD) { |
There was a problem hiding this comment.
Can we remove the named parameters here please (for sure in the DumpXxx() methods). It is just going to add one line version string to the var block. The _ map[stringUpdateWithMD in the return parameters is quite distracting.
xds/internal/client/client.go
Outdated
| mu sync.Mutex | ||
| ldsWatchers map[string]map[*watchInfo]bool | ||
| ldsVersion string |
There was a problem hiding this comment.
Is this version field only used in the CSDS response? Can we add a small comment for the same. Thanks.
There was a problem hiding this comment.
Yes, comment added.
Actually, only LDS needs the version 🤷♂️
But I added version for the other resources as well.
xds/internal/client/dump_test.go
Outdated
| compareDump(t, client.DumpEDS, nackVersion, wantDump) | ||
| } | ||
|
|
||
| func compareDump(t *testing.T, dumpFunc func() (string, map[string]UpdateWithMD), wantVersion string, wantDump interface{}) { |
There was a problem hiding this comment.
Test helper is a function that performs a setup or teardown task, such as constructing an input message, that does not depend on the code under test. See: go/go-style/decisions#mark-test-helpers
This function should ideally be changed to return an error and let the test handle it.
xds/internal/client/client.go
Outdated
| mu sync.Mutex | ||
| ldsWatchers map[string]map[*watchInfo]bool | ||
| ldsVersion string |
There was a problem hiding this comment.
Yes, comment added.
Actually, only LDS needs the version 🤷♂️
But I added version for the other resources as well.
xds/internal/client/dump.go
Outdated
| } | ||
| } | ||
|
|
||
| func (c *clientImpl) dump(t ResourceType) (version string, _ map[string]UpdateWithMD) { |
Metadata includs timestamp, status, raw message, error details.
Before, it only keeps parsed resources.
UnmarshalXXX()functions (e.g.UnmarshalListener()to return metadata)NewXXX()functions (e.g.NewListener()to handle the metadata)Dump()functions to dump message and metadataMost of the test changes are boilerplate changes due to the function signature change.
The real tests are in
dump_test.go