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.

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

ref(clickhouse): Consolidate error for native and HTTP reader/writer #813

Merged
merged 3 commits into from
Mar 5, 2020

Conversation

tkaemming
Copy link
Contributor

See #406 (comment).

Also changes the casing from ClickHouseError to ClickhouseError for consistency with the rest of the codebase.

@tkaemming tkaemming requested a review from a team March 4, 2020 21:25
@@ -92,7 +80,7 @@ def write(self, rows: Iterable[WriterTableRow]):
details = CLICKHOUSE_ERROR_RE.match(content)
if details is not None:
code, type, message = details.groups()
raise ClickHouseError(int(code), type, message)
raise ClickhouseError(int(code), message)
Copy link
Contributor

Choose a reason for hiding this comment

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

What did type provide ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only ever saw it as DB::Exception which didn't seem to provide much value.

@tkaemming tkaemming merged commit 47ffa48 into master Mar 5, 2020
@tkaemming tkaemming deleted the clickhouse-error branch March 5, 2020 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants