Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

feat: retain date when checking message tables #727

Merged
merged 2 commits into from
Nov 8, 2016
Merged

Conversation

bbangert
Copy link
Member

@bbangert bbangert commented Nov 8, 2016

Ensures the date argument is passed along with the table name
when creating the rotating message table during the
get_rotating_message_table call.

Closes #722

@codecov-io
Copy link

codecov-io commented Nov 8, 2016

Current coverage is 100% (diff: 100%)

No coverage report found for master at 7c08459.

Powered by Codecov. Last update 7c08459...c7aae0c

Copy link
Member

@pjenvey pjenvey left a comment

Choose a reason for hiding this comment

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

Can you also fix update_rotating_tables to make the call in a thread (to fully close out #727)

@@ -129,10 +129,10 @@ def make_rotating_tablename(prefix, delta=0, date=None):


def create_rotating_message_table(prefix="message", read_throughput=5,
write_throughput=5, delta=0):
write_throughput=5, delta=0, date=None):
# type: (str, int, int, int) -> Table
Copy link
Member

Choose a reason for hiding this comment

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

forgot to update the signature

Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can make create_rotating_message_table's arguments more consistent with get_rotating_message_table's while you're here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, updated to defer the call to a thread.

Ensures the date argument is passed along with the table name
when creating the rotating message table during the
get_rotating_message_table call.

Closes #722
@bbangert bbangert merged commit 04756c4 into master Nov 8, 2016
@bbangert bbangert deleted the feat/issue-722 branch November 8, 2016 21:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants