-
Notifications
You must be signed in to change notification settings - Fork 7
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.
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
Set fixed restore method for table engines and refactor #171
Set fixed restore method for table engines and refactor #171
Conversation
return self._make_table(result[0]) | ||
|
||
return None | ||
tables = self.get_tables(db_name, [table_name], short_query) |
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.
Let's add an exception if it is returned > 1
ch_backup/clickhouse/models.py
Outdated
""" | ||
A static method for determining whether an engine is replicated or not. | ||
""" | ||
return Table("", "", engine, [], [], "", "", None).is_replicated() |
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.
Must be adjusted on every change of Table
constructor.
Why not just "MergeTree" in engine and "Replicated" in engine
and reuse it in is_replicated()
ch_backup/logic/table.py
Outdated
# Fix https://github.com/ClickHouse/ClickHouse/pull/55540 is pesented since 23.8. | ||
if not maybe_table_short.is_merge_tree(): | ||
logging.debug( | ||
'Skiptable "{}.{}" data restore, because it is not MergeTree like.', |
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.
'Skiptable "{}.{}" data restore, because it is not MergeTree like.', | |
'Skip table "{}.{}" data restore, because it is not MergeTree family.', |
ch_backup/logic/table.py
Outdated
maybe_table_short = context.ch_ctl.get_table( | ||
table_meta.database, table_meta.name, short_query=True | ||
) | ||
assert ( |
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.
Let's use if
and an exception instead of assert
. maybe_table_short
can be None
, but asserts can be disabled in runtime
No description provided.