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.

Sign up for GitHub

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

Jump to bottom

Feature: Add TTL for actors #315

Merged
merged 14 commits into from Mar 31, 2022
Merged

Feature: Add TTL for actors #315

merged 14 commits into from Mar 31, 2022

Conversation

chinzhiweiblank
Copy link
Contributor

@chinzhiweiblank chinzhiweiblank commented Dec 17, 2021 •
edited

Description

Added TTL for actors: reminders and timers.

I suspect the examples are failing because this feature has been merged into master but not been released yet.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #309

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@chinzhiweiblank chinzhiweiblank requested review from a team as code owners December 17, 2021 06:19
@chinzhiweiblank chinzhiweiblank changed the title Feature/actor ttl [WIP] Feature: Add TTL for actors Dec 17, 2021
@chinzhiweiblank chinzhiweiblank marked this pull request as draft December 17, 2021 09:00
@codecov
Copy link

codecov bot commented Dec 17, 2021 •
edited

Codecov Report

Merging #315 (28fc53d) into master (a984de1) will increase coverage by 0.03%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master     #315      +/-   ##
==========================================
+ Coverage   89.01%   89.04%   +0.03%     
==========================================
  Files          55       55              
  Lines        2412     2429      +17     
==========================================
+ Hits         2147     2163      +16     
- Misses        265      266       +1
Impacted Files Coverage Δ
dapr/actor/runtime/manager.py 93.97% <ø> (ø)
dapr/actor/runtime/_reminder_data.py 97.77% <90.90%> (-2.23%) ⬇️
dapr/actor/runtime/_timer_data.py 100.00% <100.00%> (ø)
dapr/actor/runtime/actor.py 98.36% <100.00%> (ø)
dapr/actor/runtime/remindable.py 87.50% <100.00%> (+1.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a984de1...28fc53d. Read the comment docs.

@berndverst berndverst mentioned this pull request Jan 8, 2022
Merged
@berndverst
Copy link
Member

Please rebase once #326 has been merged.

berndverst
berndverst requested changes Jan 10, 2022
View reviewed changes
Copy link
Member

@berndverst berndverst left a comment

Choose a reason for hiding this comment

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

Please add the missing doc string changes.

Other than that, it would be good if you can rebase against the latest proto changes.
Once #326 has been merged you can fetch the latest master and rebase your branch against that.

@chinzhiweiblank chinzhiweiblank force-pushed the feature/actor_ttl branch 2 times, most recently from 57ba43e to fd11872 Compare January 13, 2022 15:01
@berndverst berndverst marked this pull request as ready for review January 14, 2022 02:53
@berndverst berndverst changed the title [WIP] Feature: Add TTL for actors Feature: Add TTL for actors Jan 14, 2022
@berndverst
Copy link
Member

berndverst commented Jan 14, 2022 •
edited

Something introduced in this PR is preventing receive_reminder from being called. Requires further investigation.

chinzhiweiblank and others added 7 commits March 15, 2022 11:24
@chinzhiweiblank @berndverst
Initial commit of ttl
3c10b49 
Signed-off-by: chinzhiweiblank <chinzhiweiblank@gmail.com>
Signed-off-by: root <root@DESKTOP-C475BU9.localdomain>
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@chinzhiweiblank @berndverst
Fix dictionary serialization order
251c2a4 
Signed-off-by: chinzhiweiblank <chinzhiweiblank@gmail.com>
Signed-off-by: root <root@DESKTOP-C475BU9.localdomain>
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@chinzhiweiblank @berndverst
Fix linting
0738196 
Signed-off-by: root <root@DESKTOP-C475BU9.localdomain>
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@berndverst
Remove ttl from examples
c1940f2 
Signed-off-by: root <root@DESKTOP-C475BU9.localdomain>
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@berndverst
Add back ttl
954943d 
Signed-off-by: root <root@DESKTOP-C475BU9.localdomain>
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@berndverst
Add ttl into receive_reminder
124610d 
Signed-off-by: root <root@DESKTOP-C475BU9.localdomain>
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@chinzhiweiblank @berndverst
Add docstrings
d91b978 
Signed-off-by: chinzhiweiblank <chinzhiweiblank@gmail.com>
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
chinzhiweiblank and others added 3 commits March 15, 2022 11:24
@chinzhiweiblank @berndverst
Test ttl
2670d22 
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@chinzhiweiblank @berndverst
Test with ttl
3b58588 
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@berndverst
fix linter
d92818d 
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@berndverst berndverst force-pushed the feature/actor_ttl branch 2 times, most recently from 4a17e0e to ece0860 Compare March 15, 2022 20:53
@berndverst
Make ttl optional
256fecf 
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@berndverst berndverst added this to the v1.6 milestone Mar 15, 2022
@berndverst berndverst modified the milestones: v1.6, v1.7 Mar 29, 2022
yaron2 and others added 2 commits March 29, 2022 12:07
@yaron2
Merge branch 'master' into feature/actor_ttl
0b02b25
@berndverst
make ttl optional
28fc53d 
Signed-off-by: Bernd Verst <4535280+berndverst@users.noreply.github.com>
@berndverst berndverst merged commit c0a88d8 into dapr:master Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@berndverst berndverst berndverst approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.

add support for TTL and time/interval formats in actor timers/reminders
3 participants
@chinzhiweiblank @berndverst @yaron2