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

feat: schedule job that removes expired keys #1187

Merged
merged 25 commits into from
Mar 6, 2024
Merged

feat: schedule job that removes expired keys #1187

merged 25 commits into from
Mar 6, 2024

Conversation

srieteja
Copy link
Contributor

@srieteja srieteja commented Dec 20, 2023 •
edited
Loading

closes atsign-foundation/at_server#1660
- What I did

- How I did it

  • introduced new param in AtClientPrefs 'expiryCheckTimeIntervalMins' which defines the interval at which the delete expired keys job should run. Defaults to 10 min
  • schedule the deleteExpiredKeys job while creating the persistence instances [in StorageManager]

- How to verify it

  • manual testing has been done for now to verify the updated behaviour
  • unit tests present to validate the changes

- Description for the changelog

feat: schedule job that removes expired keys

gkc
gkc requested changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

LGTM but will not approve for now, given that you are still

evaluating the possibility of writing unit tests

@srieteja srieteja mentioned this pull request Jan 4, 2024
Open
@srieteja srieteja mentioned this pull request Jan 8, 2024
Closed
@srieteja srieteja marked this pull request as draft January 9, 2024 10:41
@srieteja
Copy link
Contributor Author

srieteja commented Jan 9, 2024

This PR needs atsign-foundation/at_server#1713 merged and at_persistence_secondary published. Will wait unit that is done.

@srieteja srieteja mentioned this pull request Feb 19, 2024
Merged
@srieteja srieteja marked this pull request as ready for review February 22, 2024 09:12
@srieteja
Copy link
Contributor Author

Added unit tests. Consumed latest version of at_persistence_secondary_server v3.0.61. Ready for review

@srieteja srieteja requested a review from gkc February 22, 2024 09:13
@srieteja srieteja requested a review from gkc February 23, 2024 20:17
gkc
gkc reviewed Feb 26, 2024
View reviewed changes
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

LGTM

@srieteja srieteja requested a review from gkc March 5, 2024 08:32
@srieteja
Copy link
Contributor Author

srieteja commented Mar 5, 2024

The unit tests have been updated. Ready to be reviewed/approved

gkc
gkc approved these changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@gkc gkc left a comment

Choose a reason for hiding this comment

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

lgtm

@murali-shris murali-shris merged commit 566ca92 into trunk Mar 6, 2024
10 checks passed
@murali-shris murali-shris deleted the delete_expired_keys branch March 6, 2024 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@murali-shris murali-shris murali-shris approved these changes

@gkc gkc gkc approved these changes

@sitaram-kalluri sitaram-kalluri Awaiting requested review from sitaram-kalluri

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve commit log handling for expired (expiresAt has passed) records
3 participants
@srieteja @gkc @murali-shris