Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Cleanup use of GetClientConfigNoLock() #3197

Closed
marckhouzam opened this issue Aug 25, 2022 · 1 comment
Closed

Cleanup use of GetClientConfigNoLock() #3197

marckhouzam opened this issue Aug 25, 2022 · 1 comment
Labels
area/cli kind/bug PR/Issue related to a bug

Comments

@marckhouzam
Copy link
Contributor

Cleanup

There was a need for a quick fix for failing tests which was done in #3177.
Now that the tests are unblocked, we should probably cleanup the use of GetClientConfigNoLock() which is always preceded by calls to

AcquireTanzuConfigLock()
defer ReleaseTanzuConfigLock()

and instead use GetClientConfig() directly,

We could even consider making AcquireTanzuConfigLock() and ReleaseTanzuConfigLock() private.

There may be other aspects of that API that I don't understand, so take my suggestion with a grain of salt.

Affected product area (please put an X in all that apply)

  • (x) CLI

Version (include the SHA if the version is not obvious)

Latest main: af5a644

@marckhouzam marckhouzam added kind/bug PR/Issue related to a bug needs-triage Indicates an issue or PR needs to be triaged labels Aug 25, 2022
@rajathagasthya rajathagasthya added area/cli and removed needs-triage Indicates an issue or PR needs to be triaged labels Sep 6, 2022
@marckhouzam
Copy link
Contributor Author

This is wrong. Using GetClientConfig() directly does not retain the lock and therefore does not lock the config file past the reading of it. @anujc25 gave a good explanation here #3665 (comment)

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area/cli kind/bug PR/Issue related to a bug
Projects
None yet
Development

No branches or pull requests

2 participants