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(events): add "Raw" suffix to {Get,Subscribe}ActorEvents #11741

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Mar 19, 2024

This is done with the intention to add new {Get,Subscribe}ActorEvents in a future release (i.e. soon!) with both decoded values (dag-json represented) and simplified (no flags or codec). But because this comes with some trade-offs wrt fidelity of information (e.g. likely needing to drop events with badly encoded values, and not retaining original codec), we need to also have a Raw form of these APIs for consumers that want to take on the burden of consuming them as they are.

The changes being proposed in #11707 roughly outline the functionality of the new methods, but further work needs to be done on the return types and argument types. Rather than attempt to rush that for a v1.26.0 release, I'm proposing that we make this rename change for inclusion in v1.26.0 in order to avoid a future rename to make way for #11707 or to come up with new names for that functionality.

Related Issues

#11707

Proposed Changes

  1. Rename GetActorEvents to GetActorEventsRaw
  2. Rename SubscribeActorEvents to SubscribeActorEventsRaw

This is done with the intention to add new {Get,Subscribe}ActorEvents in a
future release (i.e. soon!) with both decoded values (dag-json represented)
and simplified (no flags or codec). But because this comes with some
trade-offs wrt fidelity of information (e.g. likely needing to drop events with
badly encoded values, and not retaining original codec), we need to also have
a Raw form of these APIs for consumers that want to take on the burden of
consuming them as they are.
Copy link
Contributor

@aarshkshah1992 aarshkshah1992 left a comment

Choose a reason for hiding this comment

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

lgtm

@rvagg rvagg merged commit 73947ea into master Mar 19, 2024
92 checks passed
@rvagg rvagg deleted the rvagg/actor-events-raw branch March 19, 2024 08:22
@rjan90 rjan90 mentioned this pull request Mar 20, 2024
8 tasks
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.

4 participants