refactor(loader): adjust LoadContext to 1.7.0 version by sadwitdastreetz · Pull Request #687 · apache/hugegraph-toolchain

sadwitdastreetz · 2025-10-29T08:58:21Z

Purpose of the PR

close #xxx

Main Changes

Verifying these changes

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

Does this PR potentially affect the following parts?

Documentation Status

Doc - TODO
Doc - Done
Doc - No Need

codecov · 2025-10-30T13:39:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.59%. Comparing base (b066b80) to head (3f8893d).
⚠️ Report is 59 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (b066b80) and HEAD (3f8893d). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (b066b80) HEAD (3f8893d)

2 1

Additional details and impacted files

@@              Coverage Diff              @@
##             master     #687       +/-   ##
=============================================
- Coverage     62.49%   51.59%   -10.91%     
+ Complexity     1903      971      -932     
=============================================
  Files           262      111      -151     
  Lines          9541     5828     -3713     
  Branches        886      750      -136     
=============================================
- Hits           5963     3007     -2956     
+ Misses         3190     2562      -628     
+ Partials        388      259      -129

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

hugegraph-loader/src/main/java/org/apache/hugegraph/loader/HugeGraphLoader.java

hugegraph-client/src/main/java/org/apache/hugegraph/api/graphs/GraphsAPI.java

imbajin · 2025-10-31T04:40:35Z

hugegraph-loader/assembly/travis/install-hugegraph-from-source.sh

-bin/init-store.sh || exit 1
+sed -i 's|gremlin.graph=org.apache.hugegraph.HugeFactory|gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy|' conf/graphs/hugegraph.properties
+sed -i 's|#auth.authenticator=.*|auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator|' conf/rest-server.properties
+sed -i 's|#auth.admin_pa=.*|auth.admin_pa=pa|' conf/rest-server.properties


⚠️ 测试代码中硬编码认证信息

在测试脚本中直接硬编码 admin/pa 凭证存在以下问题:

降低了测试的灵活性

如果服务器配置改变,测试会失败

不符合配置外部化最佳实践

建议:

使用环境变量 HUGEGRAPH_ADMIN_PASSWORD

或从配置文件读取

提供默认值作为后备

Suggested change

sed -i 's|#auth.admin_pa=.*|auth.admin_pa=pa|' conf/rest-server.properties

ADMIN_PASSWORD=${HUGEGRAPH_ADMIN_PASSWORD:-pa}

sed -i "s|#auth.admin_pa=.*|auth.admin_pa=$ADMIN_PASSWORD|" conf/rest-server.properties

echo -e "$ADMIN_PASSWORD" | bin/init-store.sh || exit 1

the username and password are already written in .properties, the sed command is only to let them show out

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/FileLoadTest.java

.github/workflows/loader-ci.yml

Copilot

Pull Request Overview

This PR enables authentication support in HugeGraph Loader tests by configuring the test environment to use StandardAuthenticator and updating all test cases to include username/password credentials ("admin"/"pa"). This brings the tests in line with a more secure, authenticated setup.

Adds authentication configuration to the Travis CI installation script for HugeGraph server
Updates all functional test classes to include authentication credentials when invoking the loader
Refactors code in GraphsAPI.java and GraphsManager.java to remove unused graphSpace-related overloads
Removes a dead System.exit(1) call in HugeGraphLoader.java that was unreachable after the return statement

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
LoadTest.java	Changed hardcoded password from "admin" to "pa" in the authmain helper method
KafkaLoadTest.java	Added ArrayList/Arrays imports and replaced HugeGraphLoader.main() calls with authmain(), added credentials to tests requiring manual argument construction
JDBCLoadTest.java	Replaced HugeGraphLoader.main() calls with authmain() to include authentication
HDFSLoadTest.java	Added ArrayList/Arrays imports and updated tests to append authentication credentials to argument lists
FileLoadTest.java	Replaced HugeGraphLoader.main() calls with authmain() and updated tests constructing loaders manually to include credentials
HugeGraphLoader.java	Removed unreachable System.exit(1) statement following a return
install-hugegraph-from-source.sh	Configured HugeGraph servers with StandardAuthenticator and set admin password to "pa"
GraphsManager.java	Removed unused graphSpace parameter overload methods
GraphsAPI.java	Simplified API methods by removing graphSpace parameter support and refactoring mode retrieval logic
loader-ci.yml	Updated HugeGraph commit ID reference

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/KafkaLoadTest.java

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/HDFSLoadTest.java

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/FileLoadTest.java

.github/workflows/loader-ci.yml

imbajin · 2025-10-31T07:32:54Z

hugegraph-loader/src/main/java/org/apache/hugegraph/loader/HugeGraphLoader.java

        try {
            loader = new HugeGraphLoader(args);
        } catch (Throwable e) {
            Printer.printError("Failed to start loading", e);


‼️ 代码逻辑问题 - 移除 System.exit(1) 可能导致测试行为不一致

移除 System.exit(1) 调用会改变程序的错误处理行为:

问题:

原代码在加载失败时会立即终止 JVM 进程(退出码 1)

新代码只是 return,程序会继续执行

影响:

在单元测试中,这可能导致测试无法正确检测失败情况

在生产环境中可能导致错误被静默忽略

建议:
如果这个改动是为了改善测试体验,建议:

考虑通过抛出异常而不是静默返回

或者添加注释说明为什么移除 System.exit(1)

确保所有调用方都能正确处理这种情况

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/LoadTest.java

imbajin · 2025-10-31T07:33:14Z

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/LoadTest.java

+     * @see HugeGraphLoader#main(String[])
+     */
    public static void authmain(String[] args) {
        ArrayList list = new ArrayList(Arrays.asList(args));


🧹 代码风格 - 可以简化泛型声明

这行代码可以使用菱形操作符简化:

Suggested change

ArrayList list = new ArrayList(Arrays.asList(args));

ArrayList list = new ArrayList<>(Arrays.asList(args));

或者更好的做法是添加类型参数:

ArrayList<String> list = new ArrayList<>(Arrays.asList(args));

这样可以:

避免原始类型警告

提供更好的类型安全性

代码更清晰易读

Copilot

Pull Request Overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-10-31T07:39:51Z

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/LoadTest.java

+     * @see HugeGraphLoader#main(String[])
+     */
    public static void authmain(String[] args) {
        ArrayList list = new ArrayList(Arrays.asList(args));


The ArrayList should use generic type parameter ArrayList<String> instead of raw type to avoid unchecked warnings and provide type safety.

Copilot · 2025-10-31T07:39:51Z

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/LoadTest.java

+     * Specifically, it appends:
+     * --username admin
+     * --password pa
+     * to the end of the original argument list before delegating to HugeGraphLoader.


The documentation states '--password pa' but should be updated to clarify this is a simplified test password. Consider documenting why this specific value was chosen or if it's a placeholder that should be changed in production environments.

Suggested change

* to the end of the original argument list before delegating to HugeGraphLoader.

* to the end of the original argument list before delegating to HugeGraphLoader.

* 

* Note: The password "pa" is a simplified test password used only for testing purposes.

* It is a placeholder and must be changed in production environments to a secure value.

* The choice of "pa" is arbitrary and intended to facilitate automated testing.

Copilot · 2025-10-31T07:39:52Z

hugegraph-loader/src/main/java/org/apache/hugegraph/loader/HugeGraphLoader.java

        } catch (Throwable e) {
            Printer.printError("Failed to start loading", e);
-            System.exit(1);
            return;


After removing System.exit(1), the method now returns normally after an error. This could cause confusion as the error is printed but execution continues in the calling context. Consider documenting this behavior change or ensuring callers check for errors appropriately.

Suggested change

return;

System.exit(1);

refactor(loader): adjust LoadContext to 1.7.0 version

c1da411

dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Oct 29, 2025

github-actions bot added the loader hugegraph-loader label Oct 29, 2025

sadwitdastreetz added 2 commits October 30, 2025 09:31

test

f4f637f

test

b36d74c

dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Oct 30, 2025

sadwitdastreetz added 2 commits October 30, 2025 09:52

test

3eade9c

test

4c2a1db

dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Oct 30, 2025

refactor(loader): add auth for tests

1eeb926

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Oct 30, 2025

sadwitdastreetz added 3 commits October 30, 2025 19:53

fix(loader): cleared tests for 1.7.0 server

0b4b33a

returned original sh for installation of HugeGraph Server

386d669

refactor(client): revert to ORIGINAL version

9444707

github-actions bot added the client hugegraph-client label Oct 30, 2025

rebuild loader-ci

7bdd211

sadwitdastreetz added 7 commits October 30, 2025 21:50

rebuild loader-ci

77aba59

rebuild loader-ci

5259c4b

rebuild loader-ci

47c0373

rebuild loader-ci

ebb64c9

rebuild loader-ci

0b61e7c

rebuild loader-ci

05b6e79

rebuild loader-ci

1c6fa16

imbajin requested a review from Copilot October 31, 2025 04:32

Update loader-ci.yml

83a0e89

imbajin reviewed Oct 31, 2025

View reviewed changes

hugegraph-loader/src/main/java/org/apache/hugegraph/loader/HugeGraphLoader.java Show resolved Hide resolved

imbajin reviewed Oct 31, 2025

View reviewed changes

hugegraph-client/src/main/java/org/apache/hugegraph/api/graphs/GraphsAPI.java Show resolved Hide resolved

imbajin reviewed Oct 31, 2025

View reviewed changes

hugegraph-client/src/main/java/org/apache/hugegraph/api/graphs/GraphsAPI.java Show resolved Hide resolved

imbajin reviewed Oct 31, 2025

View reviewed changes

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/FileLoadTest.java Outdated Show resolved Hide resolved

imbajin reviewed Oct 31, 2025

View reviewed changes

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/FileLoadTest.java Show resolved Hide resolved

imbajin reviewed Oct 31, 2025

View reviewed changes

.github/workflows/loader-ci.yml Show resolved Hide resolved

Copilot AI reviewed Oct 31, 2025

View reviewed changes

sadwitdastreetz added 3 commits October 31, 2025 13:18

fix(loader): fix updates

8960879

fix(loader): fix updates

925e81b

fix(loader): fix updates

13c67ce

imbajin requested a review from Copilot October 31, 2025 07:31

imbajin reviewed Oct 31, 2025

View reviewed changes

.github/workflows/loader-ci.yml Show resolved Hide resolved

imbajin reviewed Oct 31, 2025

View reviewed changes

hugegraph-loader/src/test/java/org/apache/hugegraph/loader/test/functional/LoadTest.java Show resolved Hide resolved

imbajin reviewed Oct 31, 2025

View reviewed changes

Copilot AI reviewed Oct 31, 2025

View reviewed changes

sadwitdastreetz added 2 commits October 31, 2025 16:51

fix(loader): fix updates

8336872

fix(loader): rename the method of authentication for loader in tests

3f8893d

imbajin approved these changes Oct 31, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 31, 2025

imbajin requested a review from Thespica October 31, 2025 10:18

Pengzna approved these changes Oct 31, 2025

View reviewed changes

Pengzna merged commit 2c4c040 into apache:master Oct 31, 2025
15 of 17 checks passed

-sed -i 's|#auth.admin_pa=.*|auth.admin_pa=pa|' conf/rest-server.properties
+ADMIN_PASSWORD=${HUGEGRAPH_ADMIN_PASSWORD:-pa}
+sed -i "s|#auth.admin_pa=.*|auth.admin_pa=$ADMIN_PASSWORD|" conf/rest-server.properties
+echo -e "$ADMIN_PASSWORD" | bin/init-store.sh || exit 1

	ArrayList list = new ArrayList(Arrays.asList(args));
	ArrayList list = new ArrayList<>(Arrays.asList(args));

-     * to the end of the original argument list before delegating to HugeGraphLoader.
+     * to the end of the original argument list before delegating to HugeGraphLoader.
+     * <p>
+     * <b>Note:</b> The password "pa" is a simplified test password used only for testing purposes.
+     * It is a placeholder and <b>must be changed</b> in production environments to a secure value.
+     * The choice of "pa" is arbitrary and intended to facilitate automated testing.

Conversation

sadwitdastreetz commented Oct 29, 2025

Purpose of the PR

Main Changes

Verifying these changes

Does this PR potentially affect the following parts?

Documentation Status

Uh oh!

codecov bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

imbajin Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

sadwitdastreetz Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

imbajin Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

imbajin Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Oct 30, 2025 •

edited

Loading