chore(ci): replace hardcoded server commit with latest release#732
Conversation
Agent-Logs-Url: https://github.com/lokidundun/hugegraph-toolchain/sessions/6d86bea0-38a3-45d5-8e70-13d5e64b21bf Co-authored-by: lokidundun <206712414+lokidundun@users.noreply.github.com>
dosubot
Bot
added
the
size:M
There was a problem hiding this comment.
Pull request overview
Updates CI workflows to follow the latest Apache HugeGraph stable release automatically (instead of pinned SHAs), and adjusts the Go Gremlin client to work with HugeGraph 1.7.0+ graph-space traversal source bindings.
Changes:
- CI: replace hardcoded
COMMIT_IDin multiple workflows with agithub-scriptstep that resolves the latest release tag to a commit SHA. - CI (loader): key the HugeGraph server cache using the resolved release commit.
- Go client: send default Gremlin
aliaseson POST requests and update Gremlin tests to useg.V().
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
hugegraph-client-go/api/v1/gremlin/gemlin.go |
Switch Gremlin POST aliases to a map and auto-inject default aliases for HugeGraph 1.7.0+. |
hugegraph-client-go/api/v1/gremlin/gemlin_test.go |
Update Gremlin test to use g.V() and improve failure reporting. |
.github/workflows/tools-ci.yml |
Resolve COMMIT_ID from latest HugeGraph release tag via actions/github-script. |
.github/workflows/spark-connector-ci.yml |
Resolve COMMIT_ID from latest HugeGraph release tag via actions/github-script. |
.github/workflows/loader-ci.yml |
Resolve COMMIT_ID dynamically and adjust server cache key/path to the resolved commit. |
.github/workflows/hubble-ci.yml |
Resolve COMMIT_ID from latest HugeGraph release tag via actions/github-script. |
.github/workflows/client-go-ci.yml |
Resolve COMMIT_ID from latest HugeGraph release tag via actions/github-script. |
.github/workflows/client-ci.yml |
Resolve COMMIT_ID from latest HugeGraph release tag via actions/github-script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { data: tag } = await github.rest.git.getTag({ owner, repo, tag_sha: sha }); | ||
| sha = tag.object.sha; | ||
| } | ||
| sha = sha.substring(0, 7); |
| const { data: tag } = await github.rest.git.getTag({ owner, repo, tag_sha: sha }); | ||
| sha = tag.object.sha; | ||
| } | ||
| sha = sha.substring(0, 7); |
| 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, | ||
| } | ||
| } |
| // Sending these aliases lets scripts use `g.V()` against the active graph | ||
| // regardless of how the server names its bindings internally. |
| t.Errorf("client.Gremlin.Post status=%d, message=%s", | ||
| respPost.StatusCode, respPost.Data.Message) |
| const { data: tag } = await github.rest.git.getTag({ owner, repo, tag_sha: sha }); | ||
| sha = tag.object.sha; | ||
| } | ||
| sha = sha.substring(0, 7); |
| const { data: tag } = await github.rest.git.getTag({ owner, repo, tag_sha: sha }); | ||
| sha = tag.object.sha; | ||
| } | ||
| sha = sha.substring(0, 7); |
| const { data: tag } = await github.rest.git.getTag({ owner, repo, tag_sha: sha }); | ||
| sha = tag.object.sha; | ||
| } | ||
| sha = sha.substring(0, 7); |
| const { data: tag } = await github.rest.git.getTag({ owner, repo, tag_sha: sha }); | ||
| sha = tag.object.sha; | ||
| } | ||
| sha = sha.substring(0, 7); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #732 +/- ##
=============================================
- Coverage 62.49% 39.34% -23.16%
- Complexity 1903 2199 +296
=============================================
Files 262 468 +206
Lines 9541 17353 +7812
Branches 886 1819 +933
=============================================
+ Hits 5963 6828 +865
- Misses 3190 10038 +6848
- Partials 388 487 +99 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| 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, | ||
| } |
| 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; | ||
| } | ||
| core.exportVariable('COMMIT_ID', sha); | ||
| core.setOutput('commit_id', sha); | ||
| console.log(`Using HugeGraph release ${release.tag_name} (${sha})`); |
| - name: Cache Maven packages | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: ~/.m2/repository | ||
| key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }} | ||
| restore-keys: ${{ runner.os }}-maven- | ||
|
|
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Overall the approach is solid: composite actions cleanly eliminate duplicated CI boilerplate, and the Go gremlin alias fix correctly addresses the 1.7.0 graph-space binding regression. Two issues need attention before merging, plus a couple of minor nits.
| with: | ||
| java-version: ${{ inputs.java-version }} | ||
| distribution: ${{ inputs.distribution }} | ||
| cache: 'maven' |
There was a problem hiding this comment.
actions/setup-java@v4 with cache: 'maven' already saves/restores ~/.m2/repository internally using a setup-java-namespaced key. The explicit Cache Maven packages step below (lines 28-33) targets the same directory with a different key, so every job saves two separate cache entries for identical content.
For workflows that previously had only one cache mechanism (tools-ci, spark-connector-ci, client-go-ci, loader-ci), this PR silently doubles the cache storage and upload time.
Remove either:
- the
cache: 'maven'line here, keeping the explicit step below, OR - the entire
- name: Cache Maven packagesblock (lines 28-33), relying solely onsetup-java's built-in caching.
Both approaches work; the latter is less YAML and the recommended pattern for setup-java@v4.
| uses: actions/setup-java@v4 | ||
| - name: Get HugeGraph stable commit id | ||
| id: get-commit | ||
| uses: ./.github/actions/get-hugegraph-commit |
There was a problem hiding this comment.
$COMMIT_ID — invisible to readers of this file
The Prepare env and service step (later in the workflow, not in the diff) still runs:
$TRAVIS_DIR/install-hugegraph-from-source.sh $COMMIT_IDCOMMIT_ID is no longer in the job env block. It works only because this composite action calls core.exportVariable('COMMIT_ID', sha) which writes to $GITHUB_ENV. A reader scanning the workflow file will see no COMMIT_ID definition and have no idea where it comes from.
All other workflows in this PR pass the SHA explicitly via steps.get-commit.outputs.commit_id. Align loader-ci to the same pattern — either convert the install step to use the setup-hugegraph-server composite action, or add an explicit env binding to that step:
- name: Prepare env and service
env:
COMMIT_ID: ${{ steps.get-commit.outputs.commit_id }}
run: |
$TRAVIS_DIR/install-hadoop.sh
$TRAVIS_DIR/install-hugegraph-from-source.sh $COMMIT_ID| uses: actions/setup-java@v3 | ||
| - name: Get HugeGraph stable commit id | ||
| id: get-commit | ||
| uses: ./.github/actions/get-hugegraph-commit |
There was a problem hiding this comment.
$COMMIT_ID issue as loader-ci
The Prepare env and service step later in this file calls:
$TRAVIS_DIR/install-hugegraph.sh $COMMIT_IDsteps.get-commit.outputs.commit_id is never referenced by name anywhere in this file — it only works via the core.exportVariable side-effect. Make the dependency explicit:
- name: Prepare env and service
env:
COMMIT_ID: ${{ steps.get-commit.outputs.commit_id }}
run: |
...
$TRAVIS_DIR/install-hugegraph.sh $COMMIT_ID| log.Fatalln(err) | ||
| } | ||
| if respPost.StatusCode != 200 { | ||
| t.Errorf("client.Gremlin.Post http_status=%d, gremlin_status=%d, message=%s", |
There was a problem hiding this comment.
🧹 Extra leading space — fails gofmt
The t.Errorf line has 5 spaces of indentation instead of a tab. gofmt will produce a diff on this file.
| t.Errorf("client.Gremlin.Post http_status=%d, gremlin_status=%d, message=%s", | |
| t.Errorf("client.Gremlin.Post http_status=%d, gremlin_status=%d, message=%s", | |
| respPost.StatusCode, respPost.Data.Status.Code, respPost.Data.Status.Message) |
| if graphSpace == "" { | ||
| graphSpace = "DEFAULT" | ||
| } | ||
| full := graphSpace + "-" + cfg.Graph |
There was a problem hiding this comment.
🧹 Unguarded empty Graph field produces a silent bad alias
If a caller constructs a client without setting Graph, full becomes "DEFAULT-" and the alias g becomes "__g_DEFAULT-". The server will accept the POST but return an error like "TraversalSource not found", which is very hard to diagnose.
Since NewCommonClient does not validate that Graph is non-empty, a small guard here makes failures explicit:
| full := graphSpace + "-" + cfg.Graph | |
| if cfg.Graph == "" { | |
| cfg.Graph = "hugegraph" | |
| } | |
| full := graphSpace + "-" + cfg.Graph |
(Or add the check to NewCommonClient and document the default.)
Purpose of the PR
Main Changes
All 6 workflows (client-ci, client-go-ci, hubble-ci, loader-ci, spark-connector-ci, tools-ci) previously had a # TODO: replace it with the (latest - n) commit id (n >= 15)
reminder with a manually-pinned COMMIT_ID. These SHAs drift out of date and silently miss server-side regressions.
Replaced the static value with an actions/github-script@v7 step that:
CI now follows the most recent stable release with zero manual upkeep.
Reused the resolved release SHA as a cache key (~/hugegraph-cache-${commit_id}) so subsequent CI runs skip the server build entirely as long as the upstream release hasn't
moved.
Server 1.7.0 introduced graph spaces and changed how the gremlin engine registers traversal sources — the graph is no longer a top-level Groovy global named , it's now
aliased as __g_DEFAULT- (where DEFAULT is the default graph space). The Java client was updated for this in GremlinManager.java; the Go client was not, so every gremlin
POST against a 1.7.0+ server failed with:
Could not rebind [g] to [__g_hugegraph] as [__g_hugegraph] not in the Graph or TraversalSource global bindings
Ported the Java behavior:
"__g_DEFAULT-"} when the caller hasn't supplied custom aliases.
JSON aliases query params due to a Jersey URI-template parser conflict on { / }.
to match Java's g.V().count() convention, and surfaced server status/message on failure for easier diagnosis.
Verifying these changes
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No Need