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

feat: add copy on write support to CEL definition types #239

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

nathanosdev
Copy link
Member

@nathanosdev nathanosdev commented Nov 30, 2023

When integrating ic-http-certification into ic-response-verification, I ran into issues with managing lifetimes because the DefaultRequestCertification and DefaultResponseCertification structs are holding references to data, instead of owning it.

This causes problems when you want to return a reference to data that is owned by the current function. Using copy on write (COW) for the properties on these structs allows us to keep the advantage of holding references to data while allowing the struct to take ownership of data in those cases where it's needed.

I split this PR out from the changes in ic-response-verification to make the PR smaller.

@nathanosdev nathanosdev self-assigned this Nov 30, 2023
@nathanosdev nathanosdev marked this pull request as ready for review November 30, 2023 12:30
@nathanosdev nathanosdev requested a review from a team as a code owner November 30, 2023 12:30
Copy link
Member

@keplervital keplervital left a comment

Choose a reason for hiding this comment

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

LGTM

@nathanosdev nathanosdev merged commit 11b0cda into main Nov 30, 2023
@nathanosdev nathanosdev deleted the nathan/add-cow-to-cel-types branch November 30, 2023 16:03
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.

2 participants