-
Notifications
You must be signed in to change notification settings - Fork 30
feat: migrate route info to expiring table #1079
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1079 +/- ##
==========================================
+ Coverage 99.71% 99.74% +0.03%
==========================================
Files 58 58
Lines 9131 9441 +310
==========================================
+ Hits 9105 9417 +312
+ Misses 26 24 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused on the last_connect changes. I should probably clarify the migration plans in my head first.
When do we plan to enable a migrate_tablename value, during the December deploy or possibly later? How long until we drop the old router table?
write_throughput=5): | ||
# type: (str, int, int) -> Table | ||
write_throughput=5, expires=False, | ||
migrate_tablename=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
migrate_tablename isn't used
# type: (Callable[[str, int, int], Table], str, int, int) -> Table | ||
def _make_table(table_func, tablename, read_throughput, write_throughput, | ||
expires=True): | ||
# type: (Callable[[str, int, int, bool], Table], str, int, int, bool) -> Table # noqa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this # noqa intended here (is it doing something for mypy? isn't it just for flake8 which ignores this comment?)
tempted to kill this func entirely since it's only used once, but maybe we'll want to use it for the new message table later. we could also assume table_func takes kwargs (they do) and make it pass through **kwargs to it
Closing this PR due to age and OBE. |
Closes: #1051