Skip to content

Adds support for TTL in Redis State Store #990

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
Merged
merged 11 commits into from
Jul 14, 2021
Merged

Adds support for TTL in Redis State Store #990

merged 11 commits into from
Jul 14, 2021

Conversation

@berndverst
Copy link
Member

@berndverst berndverst commented Jul 2, 2021 •
edited
Loading

Description

Adds support for TTL in Redis State Store

Issue reference

#306

Checklist

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

@berndverst berndverst force-pushed the master branch 2 times, most recently from 02668ec to 3c464a3 Compare July 2, 2021 19:56
@berndverst berndverst marked this pull request as ready for review July 2, 2021 20:02
@berndverst
Copy link
Member Author

berndverst commented Jul 2, 2021

@pkedy could you do an initial pass on this PR please?

@berndverst
Copy link
Member Author

berndverst commented Jul 2, 2021

References:
https://redis.io/commands/expire
https://redis.io/commands/persist

artursouza
artursouza suggested changes Jul 3, 2021
View reviewed changes
@berndverst berndverst requested a review from artursouza July 6, 2021 04:04
artursouza
artursouza reviewed Jul 6, 2021
View reviewed changes
artursouza
artursouza previously approved these changes Jul 6, 2021
View reviewed changes
This was referenced Jul 8, 2021
Closed
Merged
@berndverst berndverst requested review from a team as code owners July 12, 2021 23:43
@berndverst
Copy link
Member Author

berndverst commented Jul 13, 2021 •
edited
Loading

Tests pass locally. The cycle complexity shouldn't trigger as .golanci.yml specifies a minimum of 10, yet it seems that our Github workflow doesn't read this config file and alerts on anything. Also the these tests (and this PR) passed before I refactored this a little bit ago.

@artursouza
Copy link
Contributor

artursouza commented Jul 14, 2021

@berndverst I made the adjustment for lint to pass. Let me know how you feel about these.

@berndverst
Copy link
Member Author

berndverst commented Jul 14, 2021 •
edited
Loading

The irony is that these changes are computationally slower than what I had due to more if statements that need to be evaluated.

Of course it's fine for this PR but I think we should change the linter configuration. I like the original code better.

LGTM for now though

@codecov
Copy link

codecov bot commented Jul 14, 2021

Codecov Report

Merging #990 (b06dd9b) into master (01147e5) will increase coverage by 3.62%.
The diff coverage is 36.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #990      +/-   ##
==========================================
+ Coverage   30.59%   34.22%   +3.62%     
==========================================
  Files          85      133      +48     
  Lines        7148    10800    +3652     
==========================================
+ Hits         2187     3696    +1509     
- Misses       4724     6718    +1994     
- Partials      237      386     +149
Impacted Files Coverage Δ
bindings/alicloud/oss/oss.go 11.11% <ø> (ø)
bindings/alicloud/rocketmq/rocketmq.go 0.00% <0.00%> (ø)
bindings/apns/apns.go 88.00% <ø> (ø)
bindings/aws/dynamodb/dynamodb.go 10.52% <ø> (ø)
bindings/aws/kinesis/kinesis.go 2.61% <ø> (ø)
bindings/aws/s3/s3.go 10.81% <ø> (ø)
bindings/aws/sns/sns.go 10.52% <ø> (ø)
bindings/aws/sqs/sqs.go 6.55% <0.00%> (ø)
bindings/azure/cosmosdb/cosmosdb.go 25.71% <ø> (ø)
bindings/azure/eventgrid/eventgrid.go 3.93% <0.00%> (-0.27%) ⬇️
... and 178 more

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 c930c97...b06dd9b. Read the comment docs.

@dapr-bot dapr-bot merged commit 7df53a1 into dapr:master Jul 14, 2021
Taction added a commit to Taction/components-contrib that referenced this pull request Jul 15, 2021
@Taction
Merge branch 'master' into feat_vault_secret_store_fix
b121cc9 
* master: (23 commits)
  Adds support for TTL in Redis State Store (dapr#990)
  Adds support for TTL in Memcached State Store (PR Fixup) (dapr#1011)
  Adds support for TTL in Cassandra State Store (dapr#996)
  Return blob metadata and add list operation (dapr#999)
  Added new output binding for Cosmos Graph DB - Gremlin (dapr#885)
  Adds support for TTL in CosmosDB State Store (dapr#991)
  Delete unused code and update test (dapr#924)
  Return SystemProperties on events from EventHubs bindings and pubsub (dapr#1009)
  Implement E2E tests for zeebe (dapr#973)
  rebase (dapr#982)
  Update CODEOWNERS (dapr#987)
  ci: add test skip case (dapr#922)
  fix: Mysql should support more data types. dapr#923 (dapr#926)
  fix: Dapr runtime panic when handle Pub/Sub (dapr#3281) (dapr#967)
  switch to golang-jwt (dapr#993)
  unmarshal getBlobRetryCount as int (dapr#919)
  secretstores: support more format of azure key vault (dapr#944)
  Optimize vault secret component error output (dapr#909)
  Convert AZURE_KEYVAULT in conformance.yml a GitHub secret (dapr#1002)
  Use AzureKeyVaultName param for Azure Keyvault conformance test (dapr#975)
  ...

# Conflicts:
#	secretstores/hashicorp/vault/vault.go
@berndverst berndverst mentioned this pull request Oct 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@artursouza artursouza artursouza approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

automerge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@berndverst @artursouza @dapr-bot