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

Upgrade to recent libthrift #4365

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Conversation

andrew-nowak
Copy link
Member

@andrew-nowak andrew-nowak commented Nov 4, 2024

What does this change?

Our dependencies now bring in recent versions of libthrift, so remove the dependencyOverrides which were keeping us stuck to older versions.

This PR should fix 3 "high" vulnerabilities on snyk.

How should a reviewer test this change?

Deploy to TEST and check that all functionality performs as previously.

The only libthrift usage I'm aware of is in the Guardian-specific usage stream - content updates arriving from Crier will be in the thrift format, and we'll need to deserialize them. Try adding some images to draft Composer pieces, and ensure that the pending usage arrives, and is removed when removed from the Composer piece, etc.

How can success be measured?

Who should look at this?

Tested? Documented?

  • locally by committer
  • locally by Guardian reviewer
  • on the Guardian's TEST environment
  • relevant documentation added or amended (if needed)

only common-lib depends on libthrift, I think this override was copied accidentally, and then not bumped when common-lib's was
libs now bring in uptodate libthrift, no need for override
@andrew-nowak andrew-nowak marked this pull request as ready for review November 11, 2024 11:44
@andrew-nowak andrew-nowak requested review from a team as code owners November 11, 2024 11:44
Copy link
Contributor

@bryophyta bryophyta left a comment

Choose a reason for hiding this comment

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

I haven't tested it thoroughly myself but from the PR description it looks like you have. The changes seem well-motivated to me!

@prout-bot
Copy link

Seen on auth, usage, image-loader, metadata-editor, thrall, leases, cropper, collections, media-api, kahuna (merged by @andrew-nowak 9 minutes and 38 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants