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

refact: modify abnormal logs #1948

Merged
merged 1 commit into from
Oct 8, 2022
Merged

Conversation

xiaoleizi2016
Copy link
Contributor

@xiaoleizi2016 xiaoleizi2016 commented Aug 20, 2022

fix #1947

@@ -59,7 +59,7 @@
@Tag(name = "AccessAPI")
public class AccessAPI extends API {

private static final Logger LOG = Log.logger(RestServer.class);
Copy link
Member

@imbajin imbajin Aug 21, 2022

Choose a reason for hiding this comment

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

RestServer 相关是有一定原因和考虑意义的, 另外它可以全局调整 rest-api 的所有日志级别.

这个改不改可以看看其他同学意见

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这块我能理解,但是看已有代码也不够统一

Copy link
Member

Choose a reason for hiding this comment

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

这块我能理解,但是看已有代码也不够统一

嗯嗯后面有些修正是需要的 ✓ (那种一般是更新类名的时候没注意一起调整)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RestServer 相关是有一定原因和考虑意义的, 另外它可以全局调整 rest-api 的所有日志级别.

这个改不改可以看看其他同学意见

好像可以根据包路径配置日志的

Copy link
Member

Choose a reason for hiding this comment

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

RestServer 相关是有一定原因和考虑意义的, 另外它可以全局调整 rest-api 的所有日志级别.
这个改不改可以看看其他同学意见

好像可以根据包路径配置日志的

是可以, 当然我指的是运行时动态调整日志级别 (不过那个也可以单独修改)

这里复用一个类名是不是有什么其他历史考量呢, 需要确认一下 @javeme

@imbajin
Copy link
Member

imbajin commented Aug 21, 2022

And u'd better to run test in your local env first (make sure CI/Tests are also updated)

image

@xiaoleizi2016
Copy link
Contributor Author

And u'd better to run test in your local env first (make sure CI/Tests are also updated)

image

solved

@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #1948 (d7afcbb) into master (7274a5f) will increase coverage by 0.03%.
The diff coverage is 80.35%.

@@             Coverage Diff              @@
##             master    #1948      +/-   ##
============================================
+ Coverage     70.47%   70.50%   +0.03%     
  Complexity      724      724              
============================================
  Files           452      452              
  Lines         38984    38990       +6     
  Branches       5554     5554              
============================================
+ Hits          27473    27490      +17     
+ Misses         8814     8805       -9     
+ Partials       2697     2695       -2     
Impacted Files Coverage Δ
...n/java/com/baidu/hugegraph/api/auth/AccessAPI.java 0.00% <0.00%> (ø)
...n/java/com/baidu/hugegraph/api/auth/BelongAPI.java 0.00% <0.00%> (ø)
...in/java/com/baidu/hugegraph/api/auth/GroupAPI.java 0.00% <0.00%> (ø)
...n/java/com/baidu/hugegraph/api/auth/TargetAPI.java 0.00% <0.00%> (ø)
.../java/com/baidu/hugegraph/api/job/ComputerAPI.java 0.00% <0.00%> (ø)
...ava/com/baidu/hugegraph/api/profile/GraphsAPI.java 0.00% <0.00%> (ø)
...u/hugegraph/api/traversers/CustomizedPathsAPI.java 0.00% <0.00%> (ø)
...om/baidu/hugegraph/api/traversers/VerticesAPI.java 0.00% <0.00%> (ø)
...om/baidu/hugegraph/api/variables/VariablesAPI.java 0.00% <0.00%> (ø)
...a/com/baidu/hugegraph/license/LicenseVerifier.java 0.00% <0.00%> (ø)
... and 58 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

coderzc
coderzc previously approved these changes Aug 29, 2022
@@ -45,7 +45,7 @@

public class API {

protected static final Logger LOG = Log.logger(RestServer.class);
protected static final Logger LOG = Log.logger(API.class);
Copy link
Member

@imbajin imbajin Sep 1, 2022

Choose a reason for hiding this comment

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

Also, it would be nice if the log-framework could support a unified approach like this (I don't know if there is a way) :
Logger LOG = Log.logger(this);

@xiaoleizi2016 xiaoleizi2016 requested review from imbajin and coderzc and removed request for javeme, imbajin and coderzc September 6, 2022 03:45
@imbajin imbajin changed the title fix: modify abnormal logs(#1943) refact: modify abnormal logs Oct 8, 2022
@imbajin imbajin merged commit b699c5c into apache:master Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] 有很多不太规范的日志
4 participants