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

feature(client): support go client for hugegraph #514

Merged
merged 38 commits into from
Dec 11, 2023

Conversation

izliang
Copy link
Contributor

@izliang izliang commented Aug 28, 2023

Purpose of the PR

subtask of #512

Main Changes

Add Hugegraph-client for golang
Support REST API By This feature

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (make test).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects (typed here)

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@izliang izliang changed the title Feature go client : #512 feature : add go client Aug 28, 2023
@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (b066b80) 62.49% compared to head (22b9c3d) 62.46%.
Report is 6 commits behind head on master.

Files Patch % Lines
...n/java/org/apache/hugegraph/driver/HugeClient.java 33.33% 2 Missing ⚠️
...egraph/loader/direct/loader/HBaseDirectLoader.java 0.00% 2 Missing ⚠️
.../apache/hugegraph/loader/executor/LoadOptions.java 33.33% 2 Missing ⚠️
...java/org/apache/hugegraph/api/graph/VertexAPI.java 80.00% 1 Missing ⚠️
...ava/org/apache/hugegraph/api/graphs/GraphsAPI.java 66.66% 1 Missing ⚠️
...n/java/org/apache/hugegraph/client/RestClient.java 80.00% 0 Missing and 1 partial ⚠️
...rg/apache/hugegraph/exception/ServerException.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #514      +/-   ##
============================================
- Coverage     62.49%   62.46%   -0.04%     
+ Complexity     1903     1902       -1     
============================================
  Files           262      262              
  Lines          9541     9533       -8     
  Branches        886      886              
============================================
- Hits           5963     5955       -8     
  Misses         3190     3190              
  Partials        388      388              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hugegraph-client-go/Makefile Outdated Show resolved Hide resolved
hugegraph-client-go/Makefile Show resolved Hide resolved
hugegraph-client-go/Makefile Show resolved Hide resolved
hugegraph-client-go/Makefile Show resolved Hide resolved
hugegraph-client-go/README.en.md Outdated Show resolved Hide resolved
hugegraph-client-go/hgapi/hgapi.go Outdated Show resolved Hide resolved
hugegraph-client-go/hgapi/hgapi.go Outdated Show resolved Hide resolved
hugegraph-client-go/hgtransport/hgtransport.go Outdated Show resolved Hide resolved
hugegraph-client-go/hgtransport/hgtransport.go Outdated Show resolved Hide resolved
@javeme
Copy link
Contributor

javeme commented Sep 10, 2023

Is it still in progress?

@izliang
Copy link
Contributor Author

izliang commented Sep 17, 2023

In process

Change-Id: Id3081dd2f4a467ca570d2fc1a1be9b263878fa86
@imbajin imbajin changed the title feature : add go client feature(client): support go client for hugegraph Oct 16, 2023
@imbajin imbajin requested a review from zyxxoo October 16, 2023 09:00
izliang and others added 5 commits October 16, 2023 17:20
Change-Id: I76510813ff0ad20342ee97c802818d8f85241d62
Change-Id: I0d42d75cc3ae3ece1d9555ae35b464a8ac03915a
Change-Id: I9e97edcf81e550f97dc311e799eaa28283aa0a8b
Change-Id: Ibf441c8812f5cdd0993ceb43c80c341febf91ea6
zyxxoo
zyxxoo previously approved these changes Oct 17, 2023
@imbajin imbajin requested a review from javeme October 17, 2023 09:43
.github/workflows/client-go-ci.yml Outdated Show resolved Hide resolved
hugegraph-client-go/Makefile Outdated Show resolved Hide resolved
hugegraph-client-go/api/v1/api.version.go Outdated Show resolved Hide resolved
hugegraph-client-go/hugegraph.go Outdated Show resolved Hide resolved
pom.xml Outdated
@@ -28,7 +28,8 @@
<name>${project.artifactId}</name>
<url>https://github.com/apache/incubator-hugegraph-toolchain</url>
<description>
hugegraph-toolchain is the integration project of a series of utilities for HugeGraph, it includes 4 main modules (loader/hubble/tools/client)
hugegraph-toolchain is the integration project of a series of utilities for HugeGraph, it
includes 4 main modules (loader/hubble/tools/client)
Copy link
Contributor

Choose a reason for hiding this comment

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

seems don't need to wrap line

Copy link
Member

Choose a reason for hiding this comment

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

seems don't need to wrap line

Too long to read & break the auto-format rule, if it doesn't affect the pom attribute of the function, it is better to wrap line by 100 characters rather than put them in a LONG line

hugegraph-client-go/README.md Outdated Show resolved Hide resolved
hugegraph-client-go/README.md Outdated Show resolved Hide resolved
hugegraph-client-go/internal/model/model.go Outdated Show resolved Hide resolved
hugegraph-client-go/README.md Outdated Show resolved Hide resolved
hugegraph-client-go/README.md Outdated Show resolved Hide resolved

Choose a reason for hiding this comment

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

Go follows a convention where source files are all lower case with underscore separating multiple words. why not use api_response.go?other source files have the same question.

Copy link
Contributor Author

@izliang izliang Oct 23, 2023

Choose a reason for hiding this comment

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

Normally, in Go language, underline is used as the file separator. Here, because the API is a tree structure, English commas are used to distinguish between trees, and then underline is used for segmentation. For example: api.veretx.create.go and api.vertex.get_ by_id.go . The use of English commas and underscores is more conducive to presentation and layering.

Copy link
Member

Choose a reason for hiding this comment

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

Normally, in Go language, underline is used as the file separator. Here, because the API is a tree structure, English commas are used to distinguish between trees, and then underline is used for segmentation. For example: api.veretx.create.go and api.vertex.get_ by_id.go . The use of English commas and underscores is more conducive to presentation and layering.

@izliang @hankwenyx
should we follow the example of other graph communities? like nebula community go's sdk, seems does not use this naming style:
5810fc4c93fdf09980709d9e1f050a6

how about the following style:

  • main.go
  • api/
    • user/
      • user.go
    • product/
      • product.go
    • order/
      • order.go

hugegraph-client-go/api/v1/api.version.go Outdated Show resolved Hide resolved
hugegraph-client-go/api/v1/api.version.go Outdated Show resolved Hide resolved
Change-Id: Ida3e130c738da106e3a844ef432107df21ccc116
izliang and others added 2 commits October 23, 2023 16:39
Change-Id: Ie97c9a9336f0d25aac04d50efba4b36221fa6a08
Change-Id: Icdbded4c1553f7f4e71f569a64bf64ae84da0c8d
Change-Id: Ib1be757f2b92d33598a64ee728c9db865e3bc902
Change-Id: If504a41bb219151d4627c8917a27f849412e9827
@izliang
Copy link
Contributor Author

izliang commented Nov 3, 2023

At present, it is necessary to explore implementation solutions for the design of file directories and responsibilities for various APIs. There are currently several design options available:

  1. In the form of one API per file, this solution references the design of ElasticSerach. Code library address
  2. Using all one file and API formats, this solution references the design of Prometheus. Code library address
  3. By aggregating some APIs into a single file, this solution refers to the design of Influxdb. Code library address

@zyxxoo @hankwenyx @javeme @coderzc @Radeity could choose one

@zyxxoo
Copy link
Contributor

zyxxoo commented Nov 6, 2023

At present, it is necessary to explore implementation solutions for the design of file directories and responsibilities for various APIs. There are currently several design options available:

  1. In the form of one API per file, this solution references the design of ElasticSerach. Code library address
  2. Using all one file and API formats, this solution references the design of Prometheus. Code library address
  3. By aggregating some APIs into a single file, this solution refers to the design of Influxdb. Code library address

@zyxxoo @hankwenyx @javeme @coderzc @Radeity could choose one

Good idea, Maybe I choose 3 =.=, this style like with java style

@coderzc
Copy link
Member

coderzc commented Nov 10, 2023

At present, it is necessary to explore implementation solutions for the design of file directories and responsibilities for various APIs. There are currently several design options available:

  1. In the form of one API per file, this solution references the design of ElasticSerach. Code library address
  2. Using all one file and API formats, this solution references the design of Prometheus. Code library address
  3. By aggregating some APIs into a single file, this solution refers to the design of Influxdb. Code library address

@zyxxoo @hankwenyx @javeme @coderzc @Radeity could choose one

I also think 3nd is better.

Change-Id: I84c9e2c47d88e719c4dac58b050e9d770146be57
@javeme
Copy link
Contributor

javeme commented Nov 11, 2023

At present, it is necessary to explore implementation solutions for the design of file directories and responsibilities for various APIs. There are currently several design options available:

  1. In the form of one API per file, this solution references the design of ElasticSerach. Code library address
  2. Using all one file and API formats, this solution references the design of Prometheus. Code library address
  3. By aggregating some APIs into a single file, this solution refers to the design of Influxdb. Code library address

@zyxxoo @hankwenyx @javeme @coderzc @Radeity could choose one

I am not familiar with the Go language, you can refer to zyxxoo and coderzc's suggestions.

Change-Id: I18ee780a8c1ea385ded4b960e43d0f768d1258bd
Change-Id: I66bb1fd19deaf7e9ca27fe124461894b95cb798d
Change-Id: I096fd4589404e49e655041a04fa2bfebf3271ff2
Change-Id: I9fb0455ad3a76d2964ac5f1694123bd0bbb0b783
Change-Id: I9f7470fc1b09374193e917f4ebbc05dc2ba34611
@imbajin
Copy link
Member

imbajin commented Nov 22, 2023

izliang and others added 4 commits December 4, 2023 20:14
Change-Id: I3f4e8003ed4e905cf472080c837ac8f68bab76d2
Change-Id: I233a0ed114d8702ad0e9efcd85b058faa0ee6a35
Change-Id: Iedb5aa6977c2fbc8b021588f3662dd92d6ba527a
@qwtsc
Copy link

qwtsc commented Dec 5, 2023

I think you can try something like this openapi generator. Other parts LGTM.

imbajin and others added 2 commits December 5, 2023 20:12
Change-Id: Ie318574ac5c99572017e9e8d39f67598e6f24e0b
zyxxoo
zyxxoo previously approved these changes Dec 11, 2023
@imbajin imbajin merged commit 8133144 into apache:master Dec 11, 2023
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client hugegraph-client
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

8 participants