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

[#2084] feat(core): Add JDBC backend operations for fileset #2389

Merged
merged 66 commits into from
Mar 7, 2024

Conversation

xloya
Copy link
Contributor

@xloya xloya commented Feb 28, 2024

What changes were proposed in this pull request?

The purpose of this PR is to implement JDBC backend operation for Fileset metadata. Depend on #2384.

Why are the changes needed?

Fix: #2084

How was this patch tested?

Add unit tests to test the fileset metadata ops.

@xloya
Copy link
Contributor Author

xloya commented Mar 5, 2024

@jerryshao @yuqi1129 @qqqttt123 This PR is also ready for reviews, please take a look when you have time!

@yuqi1129
Copy link
Contributor

yuqi1129 commented Mar 6, 2024

I'm okay with the current changes.

@xloya
Copy link
Contributor Author

xloya commented Mar 6, 2024

@jerryshao @qqqttt123 Do you have some comments for this PR? Thanks.

@qqqttt123
Copy link
Contributor

qqqttt123 commented Mar 7, 2024

LGTM except for some questions.

@jerryshao
Copy link
Contributor

I have no further comment, overall LGTM. One follow-up thing we should do is to enable end 2 end test for relational entity store, otherwise it is hard to tell if it works as expected. @xloya can you please do it as a follow-up work?

@jerryshao
Copy link
Contributor

@yuqi1129 can you please also assist @xloya on how to test different entity store end to end. Currently, we hardcoded to use rocksdb, maybe we should make it configurable.

@yuqi1129
Copy link
Contributor

yuqi1129 commented Mar 7, 2024

@yuqi1129 can you please also assist @xloya on how to test different entity store end to end. Currently, we hardcoded to use rocksdb, maybe we should make it configurable.

Got it.

@xloya
Copy link
Contributor Author

xloya commented Mar 7, 2024

I have no further comment, overall LGTM. One follow-up thing we should do is to enable end 2 end test for relational entity store, otherwise it is hard to tell if it works as expected. @xloya can you please do it as a follow-up work?

Okay, I will add end-to-end testing of this piece soon.

@qqqttt123
Copy link
Contributor

I have no further suggestion.

@yuqi1129
Copy link
Contributor

yuqi1129 commented Mar 7, 2024

LGTM, I'm going to merge it.

@jerryshao jerryshao merged commit 9e18f92 into apache:main Mar 7, 2024
12 checks passed
@xloya xloya deleted the add-ops-for-fileset branch June 20, 2024 01:42
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.

[Subtask] Add JDBC Backend ops for Fileset
4 participants