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.

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

Delete non-LRU cache in SPIRE Agent #5184

Closed
wants to merge 21 commits into from

Conversation

amoore877
Copy link
Member

Pull Request check list

  • Commit conforms to CONTRIBUTING.md?
  • Proper tests/regressions included?
  • Documentation updated?

Affected functionality
Remove the original caching implementation in SPIRE Agent, as well as size configuration.

Description of change
Per plan in #4224 , perform final update actions by making LRU Cache with size 1000 the default, non-configurable implementation.

Also per #4224 , not for release until at least 1.11 based on #5150 , which deprecates the options but does not delete them, goes out in 1.10.

Which issue this PR fixes
Fixes #4224

Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
Signed-off-by: amoore877 <andrew.s.moore@uber.com>
@amoore877
Copy link
Member Author

It's always pleasing when a PR's deletions outnumber additions/modifications by 10:1 :)

@amoore877
Copy link
Member Author

hmm couple CI failures need to look at, which run here but not locally

@azdagron
Copy link
Member

azdagron commented Jun 4, 2024

Thanks for moving on this @amoore877. I think we need to hold of merging this until we open up the branch for 1.11.0 PRs. That's going to be ~3 months after we ship 1.10.0. Would you mind closing this PR until we get closer to that time?

@amoore877
Copy link
Member Author

Thanks for moving on this @amoore877. I think we need to hold of merging this until we open up the branch for 1.11.0 PRs. That's going to be ~3 months after we ship 1.10.0. Would you mind closing this PR until we get closer to that time?

oh, I was prepared to just have this be open that long or longer and keep branch up-to-date :)

does something bad happen on keeping PR open?

@azdagron
Copy link
Member

azdagron commented Jun 5, 2024

Heh, yeah, there are a few reasons we prefer to not have long-lived PRs that aren't making forward momentum:

  1. It confuses project-wide metrics tracked by the CNCF around time-to-merge for PRs (helps track project velocity and health)
  2. Bit of a nitpick, but we go through each PR in sync meetings and it gets a little old saying things like, "can't merge this one for a few more months" :)

@amoore877
Copy link
Member Author

thanks for explaining :)

@amoore877 amoore877 closed this Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable SPIRE Agent LRU Cache
2 participants