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!: bind callback subscriber/queryable to session lifetime #1334

Closed
wants to merge 7 commits into from

Conversation

wyfo
Copy link
Contributor

@wyfo wyfo commented Aug 28, 2024

To determine if the entity is callback-only, the only elegant way I've found is the rule "handler is ZST means callback-only". Unless users starts writing fancy implementations, it should be correct 100% of the time.

Session entities now uses weak references, except publishers because it would impact performances. Weak references also solves the issue of mass undeclarations before closing the session (when the session is an Arc), except for publishers.

Undeclarable trait has been refactored a little bit to better match its use in the code.

To determine if the entity is callback-only, the only elegant way I've found is the rule "handler is ZST means callback-only". Unless users starts writing fancy implementations, it should be correct 100% of the time.

Session entities now uses weak references, except publishers because it would impact performances. Weak references also solves the issue of mass undeclarations before closing the session (when the session is an `Arc`), except for publishers.

`Undeclarable` trait has been refactored a little bit to better match its use in the code.
@wyfo
Copy link
Contributor Author

wyfo commented Aug 28, 2024

Replaces #1295
@Mallets

@wyfo
Copy link
Contributor Author

wyfo commented Aug 28, 2024

@kydos

@wyfo
Copy link
Contributor Author

wyfo commented Aug 28, 2024

Idk why the test failed... ¯\_(ツ)_/¯

@Mallets Mallets changed the base branch from dev/1.0.0 to main August 30, 2024 13:19
zenoh/src/api/liveliness.rs Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Sep 2, 2024

PR missing one of the required labels: breaking-change bug dependencies documentation enhancement new feature internal

@wyfo wyfo added breaking-change Indicates that the issue implies a breaking change (be it at compile time or at runtime) enhancement Existing things could work better labels Sep 2, 2024
@wyfo
Copy link
Contributor Author

wyfo commented Sep 9, 2024

Closed in favor of #1364

@wyfo wyfo closed this Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Indicates that the issue implies a breaking change (be it at compile time or at runtime) enhancement Existing things could work better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants