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

Implement the identity host call and ctx.identity() in Rust #2085

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

gefjon
Copy link
Contributor

@gefjon gefjon commented Jan 3, 2025

Description of Changes

This PR adds ReducerContext::identity(&self) -> Identity, a method which reads the module identity. It's useful e.g. so modules can assert that scheduled reducers are being called by the scheduler, rather than by a client.
It's also a part of the API spec that fell by the wayside during 0.12.

This PR also adds a smoketest that said method does what it's supposed to, and adds a call to it in the rust-wasm-test module that I used during manual testing.

Still TODO: implement the same method in C# modules.

API and ABI breaking changes

Adds a new API and a new ABI function, but doesn't break anything.

Expected complexity level and risk

1: it's easy!

Testing

Describe any testing you've done, and any testing you'd like your reviewers to do,
so that you're confident that all the changes work as expected!

  • Wrote a new smoketest of the new functionality.
  • Manually called the assert_caller_identity_is_module_identity reducer of rust-wasm-test and watched it panic with the expected log output for a client caller.

This PR adds `ReducerContext::identity(&self) -> Identity`,
a method which reads the module identity. It's useful e.g.
so modules can assert that scheduled reducers are being called by the scheduler,
rather than by a client.
It's also a part of the API spec that fell by the wayside during 0.12.

This PR also adds a smoketest that said method does what it's supposed to,
and adds a call to it in the `rust-wasm-test` module that I used during manual testing.

Still TODO: implement the same method in C# modules.
@gefjon gefjon requested a review from Centril January 3, 2025 15:35
@gefjon gefjon self-assigned this Jan 3, 2025
Copy link
Collaborator

@coolreader18 coolreader18 left a comment

Choose a reason for hiding this comment

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

LGTM! Was gonna suggest caching it in a static _: OnceLock but host calls are quick and I don't think this is gonna be in a hot path anyway.

@gefjon
Copy link
Contributor Author

gefjon commented Jan 3, 2025

LGTM! Was gonna suggest caching it in a static _: OnceLock but host calls are quick and I don't think this is gonna be in a hot path anyway.

Yeah, I considered that, but I figure we don't care and I wanted to keep the PR simple.

@gefjon gefjon added this pull request to the merge queue Jan 3, 2025
Merged via the queue into master with commit a4b46c5 Jan 3, 2025
8 checks passed
@Centril Centril deleted the phoebe/identity-host-call branch January 7, 2025 18:39
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