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

fix(server): reinitialize the progress to set up graph auth friendly #2411

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

Z-HUANT
Copy link
Contributor

@Z-HUANT Z-HUANT commented Jan 9, 2024

Purpose of the PR

修复需要重新初始化数据库才能设置 auth 的问题
fixed #125 (comment)

Main Changes

Switch to auth-mode without removing graph data (不需要重新初始化数据库就可以设置 auth)

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:

自己测试流程如下

  1. 使用非 auth init-store 初始化图,此时无 admin 账号
  2. 使用非 auth 启动 HugeGraph,正常添加和查询数据(模拟用户操作)
  3. 使用 auth init-store 初始化图,创建 admin 账号,模拟用户运行一段时间后,数据库内有数据了后,想增加 auth
  4. 使用 auth 启动 HugeGraph,可以正常启动,权限校验正常,此时访问接口都需要账号密码
  5. 使用非 auth 启动 HugeGraph,模拟用户不想要 auth 了,此时可以正常启动,访问接口不需要账号密码

Does this PR potentially affect the following parts?

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

影响 init-store

Documentation Status

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

Doc:apache/incubator-hugegraph-doc#320

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working feature New feature tests Add or improve test cases labels Jan 9, 2024
Copy link

codecov bot commented Jan 9, 2024

Codecov Report

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

Comparison is base (7965aac) 66.23% compared to head (4c00dea) 66.22%.
Report is 1 commits behind head on master.

Files Patch % Lines
.../main/java/org/apache/hugegraph/cmd/InitStore.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2411      +/-   ##
============================================
- Coverage     66.23%   66.22%   -0.01%     
+ Complexity      830      828       -2     
============================================
  Files           511      511              
  Lines         42595    42596       +1     
  Branches       5942     5942              
============================================
- Hits          28211    28210       -1     
- Misses        11568    11572       +4     
+ Partials       2816     2814       -2     

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

@imbajin imbajin changed the title fix:Reinitializing the database is required to set up authentication. fix(server): reinitialize the progress to set up graph auth friendly Jan 9, 2024
@imbajin imbajin requested a review from javeme January 9, 2024 10:15
…raph/cmd/InitStore.java

Co-authored-by: imbajin <jin@apache.org>
javeme
javeme previously approved these changes Jan 13, 2024
Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -101,6 +101,8 @@ private static HugeGraph initGraph(String configPath) throws Exception {
LOG.info("Skip init-store due to the backend store of '{}' " +
"had been initialized", graph.name());
backendStoreInfo.checkVersion();
// Init the required information for creating the admin account (when switch from non-auth mode to auth mode)
Copy link
Contributor

Choose a reason for hiding this comment

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

please wrap line if exceed 80~100 chars

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 13, 2024
@javeme javeme merged commit 1dd0580 into apache:master Jan 17, 2024
17 of 21 checks passed
VGalaxies pushed a commit to VGalaxies/incubator-hugegraph that referenced this pull request Jan 17, 2024
Comment on lines 101 to 102
LOG.info("Skip init-store due to the backend store of '{}' " +
"had been initialized", graph.name());
Copy link
Member

Choose a reason for hiding this comment

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

@Z-HUANT seems we could move this log after L108? (due to the code logic)

could enhance it in another reviewed PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll put it in this PR later: #2408.

Z-HUANT added a commit to Z-HUANT/incubator-hugegraph that referenced this pull request Jan 23, 2024
Z-HUANT added a commit to Z-HUANT/incubator-hugegraph that referenced this pull request Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature New feature lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files. tests Add or improve test cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hugegraph authentication怎么配置和使用,使用默认配置在启动server时超时
3 participants