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

RealmManagement: Allow changing database_retention_policy and database_retention_ttl on a minor interface upgrade. #617

Open
wants to merge 1 commit into
base: release-1.0
Choose a base branch
from
Open

RealmManagement: Allow changing database_retention_policy and database_retention_ttl on a minor interface upgrade. #617

wants to merge 1 commit into from

Conversation

Annopaolo
Copy link
Collaborator

See 504.

@Annopaolo
Allow changing database_retention_policy and `database_retention_tt…
f6eaad4 
…l` on minor interface upgrade

See [504](astarte-platform#504).

Signed-off-by: Arnaldo Cesco <arnaldo.cesco@ispirata.com>
@Annopaolo Annopaolo requested review from rbino and bettio October 6, 2021 10:27
@Annopaolo Annopaolo added app:realm_management This issue or pull request is about astarte_realm_management application minor This issue causes only minor annoyance to the user labels Oct 6, 2021
@codecov
Copy link

codecov bot commented Oct 6, 2021 •
edited

Codecov Report

Merging #617 (f6eaad4) into release-1.0 (a99b3bb) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release-1.0     #617      +/-   ##
===============================================
+ Coverage        67.12%   67.13%   +0.01%     
===============================================
  Files              246      246              
  Lines             5457     5459       +2     
===============================================
+ Hits              3663     3665       +2     
  Misses            1794     1794
Impacted Files Coverage Δ
..._management/lib/astarte_realm_management/engine.ex 56.35% <100.00%> (+0.48%) ⬆️

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 fddf0a6...f6eaad4. Read the comment docs.

@Annopaolo Annopaolo changed the title Allow changing database_retention_policy and database_retention_ttl on a minor interface upgrade. RealmManagement: Allow changing database_retention_policy and database_retention_ttl on a minor interface upgrade. Oct 6, 2021
bettio
bettio requested changes Oct 25, 2021
View reviewed changes
Copy link
Contributor

@bettio bettio left a comment

Choose a reason for hiding this comment

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

This change looks good, however data that is stored before a similar interface update may remain stored with no ttl (or a longer one). We should instead discard older data.

@Annopaolo Annopaolo mentioned this pull request Oct 26, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bettio bettio bettio requested changes

@rbino rbino rbino approved these changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
app:realm_management This issue or pull request is about astarte_realm_management application minor This issue causes only minor annoyance to the user
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Annopaolo @bettio @rbino