feat(java): support multi-bases for writing database#5450
feat(java): support multi-bases for writing database#5450majin1102 merged 2 commits intolance-format:mainfrom
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
ddupg
force-pushed
the
feat/java-multi-base-writedataset
branch
from
11d3d4d to
420ff3d
Compare
|
Please @jaystarshot @jackye1995 help review this |
ddupg
force-pushed
the
feat/java-multi-base-writedataset
branch
from
3cb8bde to
f19733b
Compare
majin1102
left a comment
majin1102
left a comment
There was a problem hiding this comment.
Thanks for working on this, left some comments.
Please take a look
| "arrow-buffer", | ||
| "arrow-cast", | ||
| "arrow-data", | ||
| "arrow-ord", |
There was a problem hiding this comment.
The previous PR updated the Cargo.lock files in rust and python, but omitted java. When I executed the cargo build command, it triggered an automatic update.
Java Cargo.lock in main branch has now been updated, and I have rebased to solve this.
| .map(|v| std::time::Duration::from_secs(v as u64)) | ||
| .unwrap_or_else(|| std::time::Duration::from_secs(10)); | ||
|
|
||
| env.get_optional(initial_bases, |env, opt_obj| { |
There was a problem hiding this comment.
I think this block works well.
I wonder if we could use import_vec and make an FromJObjectWithEnv impl for BasePath which is easy to reuse in other cases. there's also a IntoJava for transforming. But I guess we don't need that yet.
| } | ||
|
|
||
| @Test | ||
| public void testCreateAndRead() throws Exception { |
There was a problem hiding this comment.
Wondering if testCreateMode is better
| } | ||
|
|
||
| @Test | ||
| public void testIsDatasetRootFlag() throws Exception { |
There was a problem hiding this comment.
Wondering if we could merge testIsDatasetRootFlag and testTargetByPathUri into the testOperation pattern tests?
There was a problem hiding this comment.
The content of these two tests overlaps, I have merged them.
Change-Id: I784b43b8bd903a122544043ebdce04a1106ba0ec
Change-Id: Ie42278965be810fd4d01036ff7cf1c98d74102c2
ddupg
force-pushed
the
feat/java-multi-base-writedataset
branch
from
759e95f to
9f98d04
Compare
jackye1995
left a comment
jackye1995
left a comment
There was a problem hiding this comment.
looks good to me, @majin1102 any further comments?
good to go |
3 participants
No description provided.