bitflicker64 commented on code in PR #785:
URL:
https://github.com/apache/hugegraph-toolchain/pull/785#discussion_r4199147041
##########
.github/actions/setup-candidate-sdk/action.yml:
##########
@@ -1,17 +1,16 @@
name: 'Setup same-source candidate SDK'
description: 'Build a locked Java 17 server reactor outside the workspace in
an isolated Maven repository'
-# TODO(apache/hugegraph#3263): temporary pre-merge source; after merge use
apache/hugegraph and a verified ASF SHA.
inputs:
repository:
description: 'GitHub owner/repository for the candidate SDK'
- default: 'hugegraph/hugegraph'
+ default: 'apache/hugegraph'
ref:
description: 'Fetchable candidate source ref'
- default: 'codex/cypher-minimal-compat'
+ default: 'master'
expected-commit:
description: 'Locked full candidate source commit'
- default: '4c162f539b906fa06dd83228e3691b77fa5b77d7'
+ default: 'd9abcd4317fb36128e7e4d209139ec7f3a28cdfc'
Review Comment:
Important: Moving the locked SDK to ASF master `d9abcd43` makes the PR's own
UTF-8 gate fail on both Server baselines. `spark-loader-api (candidate)` and
`spark-loader-api (release-1.7)` fail at this head with
`fixture-verify-vertices.log exited 1`: the vertex count is 2 and the ASCII
vertex reads back, but `getVertex(prefix + "_\u5317\u4eac")` returns NotFound
(evidence `result.json`: `vertices_verified=false`,
`edges_and_unicode_id_verified=false`). The same workflow passed at
`1d86dcb45e` while the SDK was `hugegraph/hugegraph@4c162f53`, and has failed
since `3034e650c0` switched to ASF master. The cause is in the SDK this line
selects: `AbstractRestClient.buildRequestBody` at `d9abcd43`
(`hugegraph-commons/hugegraph-common/.../rest/AbstractRestClient.java:141`)
calls `bodyContent.getBytes()`, which uses the JVM default charset, while
`4c162f53` passes an explicit charset. On Java 17 the default charset follows
the locale, and the executor here reports `file.encoding = ANSI_X3.
4-1968`. So the shaded Loader posts `\u5317\u4eac` as `??` and stores the
vertex under the wrong id with no error. That is silent data corruption for any
non-ASCII id or property loaded from a non-UTF-8 locale, and it affects every
Client/Loader built on this SDK, not just Spark. Please lock to an ASF commit
whose request body is encoded as UTF-8 (land `getBytes(StandardCharsets.UTF_8)`
upstream first if it is not on master yet), and keep this gate required and
green before merge rather than relaxing the US-ASCII run.
--
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]
