Skip to content

Use DynamoDB TTL to auto-expire old task results#5275

Open
crgwbr wants to merge 1 commit intocelery:mainfrom
crgwbr:feature_dynamodb_ttl
Open

Use DynamoDB TTL to auto-expire old task results#5275
crgwbr wants to merge 1 commit intocelery:mainfrom
crgwbr:feature_dynamodb_ttl

Conversation

@crgwbr
Copy link

@crgwbr crgwbr commented Jan 8, 2019

Description

Implement a TTL attribute on the DynamoDB result backend and configure the DB to
auto expire documents using the TTL.

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

you need to add tests for the proposed change.

Implement a TTL attribute on the DynamoDB result backend and configure the DB to
auto expire documents using the TTL.
@crgwbr crgwbr force-pushed the feature_dynamodb_ttl branch from 73fd8a3 to 3b55881 Compare January 9, 2019 19:33
@crgwbr
Copy link
Author

crgwbr commented Jan 9, 2019

@auvipy @thedrow Added tests as requested.

@auvipy
Copy link
Member

auvipy commented Jan 10, 2019

check the flake8 error

Copy link
Contributor

@georgepsarakis georgepsarakis left a comment

Choose a reason for hiding this comment

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

Nice work so far! I have noted some considerations, let me know if you need any help or clarifications.


def test_update_table_ttl_change_pending(self):
self.backend._client = MagicMock()
mock_describe_ttl = self.backend._client.describe_time_to_live = MagicMock()
Copy link
Contributor

Choose a reason for hiding this comment

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

moto supports table TTL, so you could use it here instead of creating Mock objects, as a means of functional testing, especially since the local DynamoDB does not support TTL yet.

achieved_state = current_status == expected
sleep(1)

def _update_table_ttl(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this operation should be optional, since some users may want to retain results.

self._timestamp_field.data_type: str(_now)
},
self._ttl_field.name: {
self._ttl_field.data_type: str(int(_now) + int(self.expires))
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should handle cases where expires is None/0 which disables expiration, as seen in the documentation?

What happens if you set NULL as the TTL field value, if this is possible? Would that perhaps disable expiration for this document?

Additionally, DynamoDB should be added in the documentation list of result backends supporting expiration.

@georgepsarakis
Copy link
Contributor

@crgwbr would you like to resume work here perhaps?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants