[Feat][Java] Initialize JAVA SDK: add INFO implementation #212
Conversation
|
|
| // @FFIConst | ||
| // @CXXReference | ||
| // @FFITypeAlias("std::map<std::string, GraphArchive::VertexInfo>") | ||
| // StdMap<StdString, VertexInfo> getVertexInfos(); |
There was a problem hiding this comment.
When compile with this function, met error : Cannot find a TypeDef for ....
There was a problem hiding this comment.
You may remove @FFITypeAlias(...), FastFFI will automatically map the return type. Or try std::map<std::string,GraphArchive::VertexInfo> (no spaces).
There was a problem hiding this comment.
Thanks!
Both ways can solve this problem.
|
PTAL :) @acezen @lixueclaire I think this PR is ready, but there are some problems when generating docs. In fact, I haven't changed existing docs neither add new docs. It's maybe related to incompatibility between Python3 and jinja2. We need to discuss it in our weekly meeting. |
|
hi, @Thespica, thanks for the job. Since you have added the unit tests for java SDK, I think you should include the test to CI, so we can test the java code in every PR automatically. |
|
@Thespica, thank you for your valuable contribution! In addition to the tests, I believe it would enhance the user experience if a simple example is provided to showcase the usage of the Java SDK. Perhaps, this example could be included when you add the documentation. |
Thanks for your advice! I well do it as soon as possible. |
Thanks for your advice too! I think adding examples when adding documentation is better for me. |
|
BTW，the known problem can be recorded in an issue that we can follow in the future. |
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ |
There was a problem hiding this comment.
here should have a empty line.
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ |
.github/workflows/java.yml
Outdated
| echo "########################################### checking maven ..." | ||
| mvn --version | ||
| - name: Build, test and install FastFFI |
There was a problem hiding this comment.
Do we need to build FastFFI from source? Maybe include the fastFFI in maven is enough
| branches: | ||
| - main | ||
| paths: | ||
| - 'java/**' |
There was a problem hiding this comment.
include cpp/src/** since the cpp may affect java
There was a problem hiding this comment.
- The Java CI is a liitle complicated，simplify the CI with the builtin llvm and fastFFI. I have create a java test CI, You can refer to it。
- It's better to write a simple README to tell others that how to
BUILD,RUNandTESTthe java sdk, you can refer to the README of cpp
| branches: | ||
| - main | ||
| paths: | ||
| - 'java/**' |
acezen
changed the title
java/install-llvm11.sh
Outdated
| @@ -0,0 +1,27 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Remove this file since we don't need to build llvm from the source
| <dependency> | ||
| <groupId>com.alibaba.fastffi</groupId> | ||
| <artifactId>llvm4jni-runtime</artifactId> | ||
| <version>${fastffi.revision}</version> |
There was a problem hiding this comment.
add <classifier>${os.detected.classifier}</classifier> to install correct jar with os platform.
| <dependency> | ||
| <groupId>com.alibaba.fastffi</groupId> | ||
| <artifactId>llvm</artifactId> | ||
| <version>${fastffi.revision}</version> |
There was a problem hiding this comment.
add <classifier>${os.detected.classifier}</classifier> to install correct jar with os platform.
| @@ -0,0 +1,38 @@ | |||
| # GraphAr Java | |||
|
|
|||
There was a problem hiding this comment.
Add a Working In Progress tag here to tell others that The JAVA SDK is working in progress and unfinished
Proposed changes
This PR aims to implement grpahinfo in Java with fastFFI.
Known problems
StdMapStdVctor<Property>Plan
Before 7.30
Make graphinfo's test better
7.31-8.13
8.14-8.28
Writers and test.
After 8.28
Related issue
#72