Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

📖 Fix Client Read/Write Functionality Comments #2647

Merged
merged 2 commits into from Jan 12, 2024
Merged

📖 Fix Client Read/Write Functionality Comments #2647

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dab940
Fix client-related comments
fxierh Jan 10, 2024
f29ed4e
Elaborate in which cases the client reads from a cache
fxierh Jan 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 7 additions & 5 deletions pkg/client/client.go
View file
Open in desktop
Expand Up @@ -90,10 +90,11 @@ type CacheOptions struct {
type NewClientFunc func(config *rest.Config, options Options) (Client, error)

// New returns a new Client using the provided config and Options.
// The returned client reads *and* writes directly from the server
// (it doesn't use object caches). It understands how to work with
// normal types (both custom resources and aggregated/built-in resources),
// as well as unstructured types.
// The returned client reads from a local cache or directly from the API server,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this or intentional? Did the previous client not only read directly from the apiserver?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, I see that if you set options.Cache it will read from the cache and not directly from the api server. However the default case would return a client that reads directly from the api-server.

Copy link
Contributor Author

@fxierh fxierh Jan 11, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @troy0820, thanks for the comments. The objective is to disambiguate the code comments and to reflect that the read could be performed on a local cache (and not necessarily on the API server).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain in the comment in which cases a cached client is returned? (By referring to the options passed to this function)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@inteon Thanks for pointing this out. The comment is updated accordingly. Could you please take another look ?

// and writes are always performed directly on the API server.
// (read operations may use object caches, but write operations do not).
// It understands how to work with normal types (both custom resources
// and aggregated/built-in resources), as well as unstructured types.
//
// In the case of normal types, the scheme will be used to look up the
// corresponding group, version, and kind for the given type. In the
Expand Down Expand Up @@ -210,7 +211,8 @@ func newClient(config *rest.Config, options Options) (*client, error) {

var _ Client = &client{}

// client is a client.Client that reads and writes directly from/to an API server.
// client is a client.Client configured to either read from a local cache or directly from the API server.
// Write operations are always performed directly on the API server.
// It lazily initializes new clients at the time they are used.
type client struct {
typedClient typedClient
Expand Down