Copilot commented on code in PR #732:
URL:
https://github.com/apache/hugegraph-toolchain/pull/732#discussion_r3179205938
##########
.github/workflows/tools-ci.yml:
##########
@@ -38,6 +35,27 @@ jobs:
with:
fetch-depth: 2
+ - name: Get HugeGraph stable commit id
+ id: get-commit
+ uses: actions/github-script@v7
+ with:
+ script: |
+ const owner = 'apache';
+ const repo = 'hugegraph';
+ const { data: release } = await
github.rest.repos.getLatestRelease({ owner, repo });
+ const { data: ref } = await github.rest.git.getRef({
+ owner, repo, ref: `tags/${release.tag_name}`
+ });
+ let sha = ref.object.sha;
+ if (ref.object.type === 'tag') {
+ const { data: tag } = await github.rest.git.getTag({ owner,
repo, tag_sha: sha });
+ sha = tag.object.sha;
+ }
+ sha = sha.substring(0, 7);
Review Comment:
This exports a 7-char commit SHA from the latest release tag. The downstream
`install-hugegraph-from-source.sh` clones `apache/hugegraph` with `--depth 150`
and then runs `git checkout "$COMMIT_ID"`; if the release commit isn’t present
in that shallow default-branch clone (e.g., tag on a release branch or older
than the depth), the checkout will fail.
To make CI robust, prefer exporting the full 40-char SHA and/or updating the
install script to fetch/checkout by tag explicitly (e.g., clone `--branch
<tag>` / fetch the specific ref) instead of relying on shallow history
containing the commit.
##########
.github/workflows/hubble-ci.yml:
##########
@@ -44,6 +41,27 @@ jobs:
with:
fetch-depth: 2
+ - name: Get HugeGraph stable commit id
+ id: get-commit
+ uses: actions/github-script@v7
+ with:
+ script: |
+ const owner = 'apache';
+ const repo = 'hugegraph';
+ const { data: release } = await
github.rest.repos.getLatestRelease({ owner, repo });
+ const { data: ref } = await github.rest.git.getRef({
+ owner, repo, ref: `tags/${release.tag_name}`
+ });
+ let sha = ref.object.sha;
+ if (ref.object.type === 'tag') {
+ const { data: tag } = await github.rest.git.getTag({ owner,
repo, tag_sha: sha });
+ sha = tag.object.sha;
+ }
+ sha = sha.substring(0, 7);
Review Comment:
This exports a 7-char commit SHA from the latest release tag. The downstream
install script typically clones `apache/hugegraph` with a shallow `--depth` on
the default branch and then does `git checkout "$COMMIT_ID"`; if the release
commit isn’t present in that shallow clone (e.g., tag on a release branch), the
checkout will fail.
Prefer using the full SHA and/or cloning/fetching the tag/ref explicitly in
the install script to avoid CI breakages as history grows.
##########
hugegraph-client-go/api/v1/gremlin/gemlin.go:
##########
@@ -132,6 +126,24 @@ func (g Post) WithGremlin(gremlin string) func(request
*PostRequest) {
}
}
+// buildDefaultAliases mirrors GremlinManager.java for HugeGraph 1.7.0+:
+// the server registers traversal sources as `__g_<graphSpace>-<graph>`
+// (graphSpace defaults to "DEFAULT", matching
HugeClientBuilder.DEFAULT_GRAPHSPACE).
+// Sending these aliases lets scripts use `g.V()` against the active graph
+// regardless of how the server names its bindings internally.
+func buildDefaultAliases(transport api.Transport) map[string]string {
+ cfg := transport.GetConfig()
+ graphSpace := cfg.GraphSpace
+ if graphSpace == "" {
+ graphSpace = "DEFAULT"
+ }
+ full := graphSpace + "-" + cfg.Graph
+ return map[string]string{
+ "graph": full,
+ "g": "__g_" + full,
+ }
+}
Review Comment:
`buildDefaultAliases()` reads `transport.GetConfig().GraphSpace`, but the
primary client constructor (`hugegraph.NewCommonClient`) currently never passes
`GraphSpace` into `hgtransport.New(...)`. As a result, setting
`Config.GraphSpace` on the Go client won’t affect the aliases and requests will
always use the default.
Fix by wiring `GraphSpace` through to the transport config (and/or ensuring
the transport used by Gremlin has `GraphSpace` set) so non-default graph spaces
work as intended.
##########
hugegraph-client-go/api/v1/gremlin/gemlin.go:
##########
@@ -132,6 +126,24 @@ func (g Post) WithGremlin(gremlin string) func(request
*PostRequest) {
}
}
+// buildDefaultAliases mirrors GremlinManager.java for HugeGraph 1.7.0+:
+// the server registers traversal sources as `__g_<graphSpace>-<graph>`
+// (graphSpace defaults to "DEFAULT", matching
HugeClientBuilder.DEFAULT_GRAPHSPACE).
+// Sending these aliases lets scripts use `g.V()` against the active graph
+// regardless of how the server names its bindings internally.
Review Comment:
The comment says default aliases are used “when the caller hasn’t supplied
custom aliases”, but there is currently no request option/helper to set
`PostRequest.aliases` (or to intentionally disable alias injection). This makes
the behavior effectively mandatory and the comment misleading.
Either add an option like `WithAliases(...)` (and possibly a way to disable
auto-aliasing), or adjust the comment to reflect the actual behavior.
##########
.github/workflows/spark-connector-ci.yml:
##########
@@ -37,6 +35,27 @@ jobs:
with:
fetch-depth: 2
+ - name: Get HugeGraph stable commit id
+ id: get-commit
+ uses: actions/github-script@v7
+ with:
+ script: |
+ const owner = 'apache';
+ const repo = 'hugegraph';
+ const { data: release } = await
github.rest.repos.getLatestRelease({ owner, repo });
+ const { data: ref } = await github.rest.git.getRef({
+ owner, repo, ref: `tags/${release.tag_name}`
+ });
+ let sha = ref.object.sha;
+ if (ref.object.type === 'tag') {
+ const { data: tag } = await github.rest.git.getTag({ owner,
repo, tag_sha: sha });
+ sha = tag.object.sha;
+ }
+ sha = sha.substring(0, 7);
Review Comment:
This exports a 7-char commit SHA from the latest release tag. The downstream
`install-hugegraph-from-source.sh` clones `apache/hugegraph` with `--depth 150`
and then runs `git checkout "$COMMIT_ID"`; if the release commit isn’t present
in that shallow default-branch clone (e.g., tag on a release branch or older
than the depth), the checkout will fail.
To make CI robust, prefer exporting the full SHA and/or changing the install
script to clone/fetch the tag/ref explicitly rather than relying on the shallow
clone containing the commit.
##########
hugegraph-client-go/api/v1/gremlin/gemlin_test.go:
##########
@@ -31,22 +31,17 @@ func TestGremlin(t *testing.T) {
if err != nil {
log.Println(err)
}
- respGet, err := client.Gremlin.Get(
- client.Gremlin.Get.WithGremlin("hugegraph.traversal().V().limit(3)"),
- )
- if err != nil {
- log.Fatalln(err)
- }
- if respGet.StatusCode != 200 {
- t.Error("client.Gremlin.GremlinGet error ")
- }
respPost, err := client.Gremlin.Post(
- client.Gremlin.Post.WithGremlin("hugegraph.traversal().V().limit(3)"),
+ client.Gremlin.Post.WithGremlin("g.V().limit(3)"),
)
if err != nil {
log.Fatalln(err)
}
+ if respPost.StatusCode != 200 {
+ t.Errorf("client.Gremlin.Post status=%d, message=%s",
+ respPost.StatusCode, respPost.Data.Message)
Review Comment:
`PostResponseData` already exposes the Gremlin status message as
`Data.Status.Message`; using `respPost.Data.Message` here often results in an
empty string (since the server typically reports errors/success under
`status.message`).
Consider reporting `respPost.Data.Status.Message` (and/or `Status.Code`) in
the failure message so test failures are diagnosable.
##########
.github/workflows/client-go-ci.yml:
##########
@@ -37,6 +34,25 @@ jobs:
with:
fetch-depth: 2
+ - name: Get HugeGraph stable commit id
+ uses: actions/github-script@v7
+ with:
+ script: |
+ const owner = 'apache';
+ const repo = 'hugegraph';
+ const { data: release } = await
github.rest.repos.getLatestRelease({ owner, repo });
+ const { data: ref } = await github.rest.git.getRef({
+ owner, repo, ref: `tags/${release.tag_name}`
+ });
+ let sha = ref.object.sha;
+ if (ref.object.type === 'tag') {
+ const { data: tag } = await github.rest.git.getTag({ owner,
repo, tag_sha: sha });
+ sha = tag.object.sha;
+ }
+ sha = sha.substring(0, 7);
Review Comment:
This exports a 7-char commit SHA from the latest release tag. The downstream
`install-hugegraph-from-source.sh` clones `apache/hugegraph` shallowly and then
runs `git checkout "$COMMIT_ID"`; if the release commit isn’t in that shallow
default-branch clone (e.g., tag points to a release branch / older commit), the
checkout will fail.
Consider exporting the full SHA and/or updating the install script to
clone/fetch the tag/ref explicitly rather than relying on shallow history
containing the commit.
##########
.github/workflows/loader-ci.yml:
##########
@@ -50,6 +47,27 @@ jobs:
with:
fetch-depth: 2
+ - name: Get HugeGraph stable commit id
+ id: get-commit
+ uses: actions/github-script@v7
+ with:
+ script: |
+ const owner = 'apache';
+ const repo = 'hugegraph';
+ const { data: release } = await
github.rest.repos.getLatestRelease({ owner, repo });
+ const { data: ref } = await github.rest.git.getRef({
+ owner, repo, ref: `tags/${release.tag_name}`
+ });
+ let sha = ref.object.sha;
+ if (ref.object.type === 'tag') {
+ const { data: tag } = await github.rest.git.getTag({ owner,
repo, tag_sha: sha });
+ sha = tag.object.sha;
+ }
+ sha = sha.substring(0, 7);
Review Comment:
This exports a 7-char commit SHA from the latest release tag. The install
scripts invoked later typically do `git clone --depth 150` of the default
branch and then `git checkout "$COMMIT_ID"`; if the release commit isn’t
contained in that shallow clone (e.g., tag points at a release branch or older
than the depth), the checkout will fail.
Consider exporting the full SHA and/or updating the install script to
clone/fetch the tag/ref explicitly so CI doesn’t depend on shallow history
including the release commit.
##########
.github/workflows/client-ci.yml:
##########
@@ -39,6 +36,27 @@ jobs:
with:
fetch-depth: 2
+ - name: Get HugeGraph stable commit id
+ id: get-commit
+ uses: actions/github-script@v7
+ with:
+ script: |
+ const owner = 'apache';
+ const repo = 'hugegraph';
+ const { data: release } = await
github.rest.repos.getLatestRelease({ owner, repo });
+ const { data: ref } = await github.rest.git.getRef({
+ owner, repo, ref: `tags/${release.tag_name}`
+ });
+ let sha = ref.object.sha;
+ if (ref.object.type === 'tag') {
+ const { data: tag } = await github.rest.git.getTag({ owner,
repo, tag_sha: sha });
+ sha = tag.object.sha;
+ }
+ sha = sha.substring(0, 7);
Review Comment:
This exports a 7-char commit SHA from the latest release tag. The downstream
`install-hugegraph-from-source.sh` clones `apache/hugegraph` with `--depth 150`
and then does `git checkout "$COMMIT_ID"`; if the release commit isn’t present
in that shallow default-branch clone (e.g., tag on a release branch or older
than the depth), the checkout will fail.
To make CI reliable, consider exporting the full SHA and/or updating the
install script to clone/fetch the tag/ref explicitly instead of relying on
shallow history containing the commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
