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

build(deps): bump github.com/jellydator/ttlcache/v2 from 2.11.1 to 3.0.1 #1099

Merged
merged 3 commits into from
Apr 21, 2023

Conversation

siretart
Copy link
Contributor

Summary

I'm trying to package this library in Debian, so that it can used by container tools like podman and friends. I've packaged most of the dependencies, but while packaging https://github.com/jellydator/ttlcache/ I made the mistake of packaging version 3. In order to not have to downgrade the Debian package, I've decided to send you this PR to move forward to version 3. :-)

Release Note

Documentation

@haydentherapper
Copy link
Contributor

Thank you for making these changes! I or one of the other maintainers will take a closer look next week. If you see any gaps in unit or e2e tests that would exercise the cache, it would be helpful to add those too.

@siretart
Copy link
Contributor Author

Awesome, that's great to hear.

I'm struggling with running the e2e tests locally. For some reason, docker-compose is not playing well with podman on my laptop. Therefore, it's not obvious to me why the aws e2e test is not working. Any ideas?

@haydentherapper
Copy link
Contributor

cmk, err = a.getCMK(ctx)
return cmk.PublicKey, err

Looks like it's panicking because cmk is nil. I'd add a "if err != nil { return nil err}" and see what error is occurring

@siretart
Copy link
Contributor Author

@haydentherapper I (finally) gotten the docker-compose setup to work with my podman installation (was some sillyness on my side) and found the bug. Now it is passing for me locally.

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Thanks so much for making this change! Only question is if we should standardize on where the loader gets set, either on initialization (like in the GCP client) or on access (in the other clients). To me, it's probably cleaner to set it once on initialization.

pkg/signature/kms/aws/signer.go Show resolved Hide resolved
pkg/signature/kms/hashivault/client.go Show resolved Hide resolved
pkg/signature/kms/azure/client.go Show resolved Hide resolved
pkg/signature/kms/aws/client.go Show resolved Hide resolved
@siretart siretart force-pushed the jellydator branch 3 times, most recently from 230217f to 5be84db Compare April 21, 2023 10:37
Signed-off-by: Reinhard Tartler <siretart@tauware.de>
@haydentherapper
Copy link
Contributor

AWS failure is unrelated to PR

@haydentherapper
Copy link
Contributor

Just one linter error!

Signed-off-by: Reinhard Tartler <siretart@tauware.de>
Signed-off-by: Reinhard Tartler <siretart@tauware.de>
@siretart
Copy link
Contributor Author

I don't get the azure e2e failure, that is, that pass passes for me. Is it possible that the test reaches out to the internet?

Copy link
Member

@bobcallaway bobcallaway left a comment

Choose a reason for hiding this comment

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

thanks!

@haydentherapper
Copy link
Contributor

Looks good now! Thank you again for making these changes!

@haydentherapper haydentherapper merged commit ac4c11c into sigstore:main Apr 21, 2023
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.

None yet

3 participants