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

Partial implementation of cache API #454

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

cceckman-at-fastly
Copy link
Contributor

The minimum possible implementation. Limitations:

  • Only insert and lookup; no transactions
  • Just key -> body; no TTL, eviction, etc.; no options

Since the insert/lookup options are not supported, the insert and lookup methods still return unavailable by default. They are enabled at runtime by ENABLE_EXPERIMENTAL_CACHE_API=1 in the environment.

`CollectingBody` reads from a `Body` and:

- Stores the data and trailers written, for future delivery in cache
  lookups
- Offers a `read() -> Body` endpoint. The read can proceed concurrently
  with writes, or after the write has completed.
}

proptest! {
#[test]
Copy link

Choose a reason for hiding this comment

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

Is #[tokio::test] not usable inside proptest!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, yeah; both of them are doing Things to the signatures of the relevant functions, and they aren't orthogonal. Added a comment.

@cceckman-at-fastly cceckman-at-fastly force-pushed the cceckman/cache-nontransact branch from 53bac42 to b9bae09 Compare March 12, 2025 20:34
Missed the "skip already read chunks" step.
Implement the non-transactional API with many limitations:
- No streaming bodies (which also makes the test brittle)
- No options processing
Some of the core cache API hostcalls are incomplete; namely, `insert`
and `lookup` don't respect the "option" arguments. We don't want to
enable access to those APIs in a way that will misbehave (e.g. ignoring
arguments).

Guard the use (and testing) of the incomplete APIs behind an environment
variable, ENABLE_EXPERIMENTAL_CACHE_API. When present, we enable the API
and run the tests.
This covers the stream-through-cache case.

Include a note on why we can't exercise "input error from stream" in an
integration test.
@cceckman-at-fastly cceckman-at-fastly force-pushed the cceckman/cache-nontransact branch from b9bae09 to 8d0cd45 Compare March 12, 2025 21:40
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