-
Notifications
You must be signed in to change notification settings - Fork 379
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
[#2551] feat(catalog-jdbc-doris): Add a basic skeleton for Doris catalog #2554
Conversation
3cef07d
to
e2c4cda
Compare
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.
LGTM
testImplementation(libs.testcontainers) | ||
testImplementation(libs.testcontainers.mysql) | ||
|
||
testRuntimeOnly(libs.junit.jupiter.engine) |
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.
We can remove the dependencies listed above because they are not currently in use.
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.
removed
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.
If we need to remove it, I believe other dependencies like test container should also be dropped.
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.
@yuqi1129 yes,I had remove all test dependencies
import com.datastrato.gravitino.exceptions.GravitinoRuntimeException; | ||
import java.sql.SQLException; | ||
|
||
/** Exception converter to gravitino exception for Doris. */ |
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.
gravitino
-> Gravitino
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.
same typo in MysqlExceptionConverter
, i can fix it in another PR (for #2553 )
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.
LGTM, thanks @zhoukangcn for your contribution.
…s catalog (apache#2554) ### What changes were proposed in this pull request? Add a basic skeleton for Doris catalog ### Why are the changes needed? Fix: apache#2551 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? just build
What changes were proposed in this pull request?
Add a basic skeleton for Doris catalog
Why are the changes needed?
Fix: #2551
Does this PR introduce any user-facing change?
N/A
How was this patch tested?
just build