Skip to content

Conversation

ItamarYuran
Copy link
Contributor

@ItamarYuran ItamarYuran commented Sep 3, 2025

Closes #9480

Got rid of a lot of mess. no sync.Once nor global parameters. I mistakenly thought that abuse commands use separate clients for each iteration but was wrong.

@ItamarYuran ItamarYuran added exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached labels Sep 3, 2025
Copy link
Contributor

@Isan-Rivkin Isan-Rivkin left a comment

Choose a reason for hiding this comment

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

I started review and then stopped in the middle.

  1. Im not sure what is the goal of this pr? it removes "once" related stuff but doesn't offer any alternative
  2. capacity

Comment on lines 17 to 18
LakectlDirName = ".lakectl"
CacheDirName = "cache"
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. What's the purpose of those directories? Why not just ~/.lakectl_token_cache.json? Or just ~/.lakectl/token_cache.json (If you really need a dir)
  2. This is not the right location for such things. It belongs to lakectl code parts, not here.

Comment on lines 31 to 38
if baseDir == "" {
var err error
baseDir, err = os.UserHomeDir()
if err != nil {
return nil, err
}
}
cachePath := filepath.Join(baseDir, lakectlDir, cacheDir)
cachePath := filepath.Join(baseDir, LakectlDirName, CacheDirName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Those sub diretories are irrelevant for this component.
It doesn't need to force or even know about LakectlDirName, CacheDirName

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached
Projects
None yet
Development

Successfully merging this pull request may close these issues.

enhance IAM auth JWT caching
2 participants