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

[bugfix]auto config timezone for Jackson #2197

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

cdphantom
Copy link

@cdphantom cdphantom commented Jul 4, 2024

What's changed?

Checklist

  • I have read the Contributing Guide
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Add or update API

  • I have added the necessary e2e tests and all cases have passed.

@tomsun28 tomsun28 self-requested a review July 5, 2024 02:12
}
LocalTime silentStart = alertSilence.getPeriodStart().toLocalTime();
LocalTime silentEnd = alertSilence.getPeriodEnd().toLocalTime();
// 判断是否为静默时间段
Copy link
Member

Choose a reason for hiding this comment

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

Should be translated into Chinese.

@tomsun28
Copy link
Contributor

tomsun28 commented Jul 7, 2024

cc @Calvin979 hi, please help take a review if have time.

@@ -23,6 +23,7 @@ spring:
static-path-pattern: /**
jackson:
default-property-inclusion: ALWAYS
time-zone: ${TZ:Asia/Shanghai}
Copy link
Contributor

Choose a reason for hiding this comment

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

due the #2122 support config time-zone in webui, can this be removed? @Calvin979 @cdphantom

Copy link
Contributor

Choose a reason for hiding this comment

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

due the #2122 support config time-zone in webui, can this be removed? @Calvin979 @cdphantom

Yes, it can be removed. @cdphantom

Comment on lines +134 to +146
private boolean isSilentPeriod(LocalTime silentStart, LocalTime silentEnd) {
if (null == silentStart || null == silentEnd) {
return false;
}
LocalTime nowLocalTime = ZonedDateTime.now().toLocalTime();
log.info("nowLocalTime:{}, silentStart:{}, silentEnd:{}, SystemDefaultTimeZoneId:{}", nowLocalTime, silentStart, silentEnd, ZoneId.systemDefault());
// 如果静默结束时间小于静默开始时间,意味着静默期跨越了午夜
if (silentEnd.isBefore(silentStart)) {
// 当前时间在午夜之前且大于等于静默开始时间,或者在午夜之后且小于静默结束时间
return nowLocalTime.isAfter(silentStart) || nowLocalTime.isBefore(silentEnd);
} else {
// 当前时间在静默开始和结束时间之间
return nowLocalTime.isAfter(silentStart) && nowLocalTime.isBefore(silentEnd);
Copy link
Contributor

Choose a reason for hiding this comment

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

hi the code comments need to be in English, and suggest use log.debug or trace instead of log.info("nowLocalTime to aviod too many repeated, unimportant logs.

@Calvin979
Copy link
Contributor

cc @Calvin979 hi, please help take a review if have time.

@tomsun28 The coding is right. It can apply to cross-day configuration as well.
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants