You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/05/29 16:48:07 UTC
[GitHub] [dolphinscheduler] 106umao opened a new pull request, #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
106umao opened a new pull request, #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284
<!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
## Purpose of the pull request
Fixed #10282
## Brief change log
Add static method `setTimeZone(TimeZone timeZone)` for class `org.apache.dolphinscheduler.common.utils.JSONUtils`
The purpose of the method is to support dynamic modification of the TimeZone setting of the ObjectMapper object
## Verify this pull request
<!--*(Please pick either of the following options)*-->
This pull request is already covered by existing tests, such as *(please describe tests)*.
method `org.apache.dolphinscheduler.common.utils.JSONUtilsTest#dateToString`
method `org.apache.dolphinscheduler.common.utils.JSONUtilsTest#stringToDate`
This change added tests and can be verified as follows:
Run `mvn clean test` in terminal
<!--*(example:)*
- *Added dolphinscheduler-dao tests for end-to-end.*
- *Added CronUtilsTest to verify the change.*
- *Manually verified the change by testing locally.* -->
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#discussion_r893127646
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/JSONUtils.java:
##########
@@ -82,6 +82,12 @@ private JSONUtils() {
throw new UnsupportedOperationException("Construct JSONUtils");
}
+ public static synchronized boolean setTimeZone(TimeZone timeZone) {
+ objectMapper.setTimeZone(timeZone);
+ logger.info("set timezone: {}", timeZone);
Review Comment:
remove if not use.
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/JSONUtils.java:
##########
@@ -82,6 +82,12 @@ private JSONUtils() {
throw new UnsupportedOperationException("Construct JSONUtils");
}
+ public static synchronized boolean setTimeZone(TimeZone timeZone) {
Review Comment:
```suggestion
public static synchronized void setTimeZone(TimeZone timeZone) {
```
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] 106umao commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
106umao commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1140830160
Hi @SbloodyS @zhuangchong
Can anyone help me review？
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] 106umao commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
106umao commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1150123650
@SbloodyS
This time I changed the time zone of the system
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] 106umao commented on a diff in pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
106umao commented on code in PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#discussion_r894716922
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/JSONUtils.java:
##########
@@ -82,6 +82,12 @@ private JSONUtils() {
throw new UnsupportedOperationException("Construct JSONUtils");
}
+ public static synchronized boolean setTimeZone(TimeZone timeZone) {
+ objectMapper.setTimeZone(timeZone);
+ logger.info("set timezone: {}", timeZone);
Review Comment:
> remove if not use.
Do not understand this sentence, please elaborate
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1140488580
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10284)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10284&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10284&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10284&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10284&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1158385257
Hi @106umao , I have noticed that many CIS fail for this reason. So I helped you solve the conflict and optimize it.Hoped you don't mind.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] SbloodyS commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
SbloodyS commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1149581865
> @zhongjiajie @caishunfeng Hi, it is still fail. 
The reason is still failed is that the github action's host machine's timezone is UTC by default. But your local timezone is not.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1146990390
The issue had been fixed #10345
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1150736879
> > @zhongjiajie @caishunfeng Hi, it is still fail. 
>
> The reason of still failed is that the github action's host machine's timezone is UTC by default. But your local timezone is not.
As @SbloodyS said, I think we need this pr. @106umao please solve the conflicts, thanks.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1158402866
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=10284)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=10284&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10284&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10284&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10284&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=10284&metric=new_duplicated_lines_density&view=list)
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] 106umao commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
106umao commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1149575625
@zhongjiajie
@caishunfeng
Hi, it is still fail.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1140487402
# [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10284?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10284](https://codecov.io/gh/apache/dolphinscheduler/pull/10284?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8e372fa) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/62bd9d2c4c0ee110687d1454a34209a77f175d94?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62bd9d2) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## dev #10284 +/- ##
============================================
- Coverage 41.15% 41.14% -0.01%
+ Complexity 4760 4759 -1
============================================
Files 855 855
Lines 34618 34621 +3
Branches 3826 3826
============================================
Hits 14246 14246
- Misses 18998 19003 +5
+ Partials 1374 1372 -2
```
| [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/10284?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pache/dolphinscheduler/common/utils/JSONUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10284/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0pTT05VdGlscy5qYXZh) | `70.10% <100.00%> (+0.95%)` | :arrow_up: |
| [...org/apache/dolphinscheduler/remote/utils/Host.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10284/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL3V0aWxzL0hvc3QuamF2YQ==) | `37.77% <0.00%> (-2.23%)` | :arrow_down: |
| [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/10284/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `52.11% <0.00%> (-1.41%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10284?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/10284?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [62bd9d2...8e372fa](https://codecov.io/gh/apache/dolphinscheduler/pull/10284?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] zhongjiajie commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
zhongjiajie commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1149509829
> The issue had been fixed #10345
So we do not need this anymore, is it?
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng merged pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
caishunfeng merged PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] 106umao commented on pull request #10284: [Fix] Fix the JSONUtils tool class time zone problem #10282
Posted by GitBox <gi...@apache.org>.
106umao commented on PR #10284:
URL: https://github.com/apache/dolphinscheduler/pull/10284#issuecomment-1158966940
> Hi @106umao , I have noticed that many CI failed for this reason. So I helped you solve the conflict and optimize it. Hoped you don't mind.
@SbloodyS @caishunfeng thx.
--
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: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org