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

Expire schemas after 7 days #5112

Merged
merged 11 commits into from
Sep 1, 2023
Merged

Expire schemas after 7 days #5112

merged 11 commits into from
Sep 1, 2023

Conversation

rauchy
Copy link
Contributor

@rauchy rauchy commented Aug 17, 2020

What type of PR is this? (check all applicable)

  • Bug Fix

Description

Schema keys get replaced only when they are refreshed. If refresh fails, or if data sources stop working - schemas would linger in Redis indefinitely.

This PR expires them 7 days after the next time they are supposedly scheduled to get refreshed. If they are refreshed by then - the TTL would slide 7 days into the future, otherwise they will expire.

@rauchy rauchy requested a review from arikfr August 17, 2020 19:36
@konnectr
Copy link
Collaborator

@rauchy thanks for the PR! We've updated a lot of things now that we're Community-driven so - if you're still interested in getting this merged - would you mind rebasing off master to re-run the CI, as well as updating merge conflicts?

We're trying to clean up our PR todo list, so if you're not interested, that's fine - we'll close the PR in about a week if we don't hear back. If you're interested in reopening the PR afterwards, we would also very much welcome that.

@konnectr
Copy link
Collaborator

It sounds like this PR needs to resolve conflicts and test

@konnectr konnectr added the good first pr A fairly simple PR, suitable for people learning Redash development label Jul 30, 2023
@codecov
Copy link

codecov bot commented Jul 31, 2023

Codecov Report

Merging #5112 (a51d6d4) into master (528807f) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5112      +/-   ##
==========================================
+ Coverage   60.84%   60.87%   +0.02%     
==========================================
  Files         155      155              
  Lines       12714    12715       +1     
  Branches     1728     1728              
==========================================
+ Hits         7736     7740       +4     
+ Misses       4750     4747       -3     
  Partials      228      228              
Files Changed Coverage Δ
redash/models/__init__.py 92.35% <100.00%> (+0.39%) ⬆️

@konnectr
Copy link
Collaborator

for good it would be necessary to add a test

@guidopetri guidopetri self-assigned this Aug 25, 2023
@guidopetri guidopetri enabled auto-merge (squash) September 1, 2023 00:56
@guidopetri guidopetri merged commit fcbe726 into master Sep 1, 2023
15 checks passed
@justinclift justinclift deleted the expire-schemas branch September 1, 2023 02:13
@guidopetri
Copy link
Contributor

@rauchy , thanks for the contribution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first pr A fairly simple PR, suitable for people learning Redash development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants