Fix lazy error handling of cron parser
What does this MR do?
This MR is for fixing the following bugs.
- If cron syntax is rufus-scheduler syntax(e.g.
every 3h
), 500 occurs. https://gitlab.com/gitlab-org/gitlab-ce/issues/31274#note_28261983 - If Gitlab timezone is invalid for
#next_time_from
, 500 occurs. https://gitlab.com/gitlab-org/gitlab-ce/issues/31274#note_28274299
Are there points in the code the reviewer needs to double check?
Why was this MR needed?
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master
(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Closes #31274 (closed)
Merge request reports
Activity
- Resolved by Shinya Maeda
- Resolved by Grzegorz Bizon
- Resolved by Grzegorz Bizon
- Resolved by Shinya Maeda
@dosuken123 Thanks for handling this bug like a pro
I review that soon (traveling now ). /cc @ayufanassigned to @grzesiek
added 120 commits
-
d670dad3...ced7212e - 118 commits from branch
gitlab-org:master
- 5231fb20 - Fix lazy error handling of cron parser
- 30c4405f - Add feature test, which reproduced 500
-
d670dad3...ced7212e - 118 commits from branch
added 1 commit
- 7202e8e4 - Extract tzinfo from Rails timezone(Fix for servertimezone ailiases)
- Resolved by Shinya Maeda
added 75 commits
-
7202e8e4...479d21d5 - 70 commits from branch
gitlab-org:master
- f473cd5f - Fix lazy error handling of cron parser
- 96be6e8a - Add feature test, which reproduced 500
- 7b55879c - Extract tzinfo from Rails timezone(Fix for servertimezone ailiases)
- a0759b39 - Add changelog
- c324eb6d - Add spec for sidekiq-cron dependency
Toggle commit list-
7202e8e4...479d21d5 - 70 commits from branch
added 1 commit
- ee957233 - Upgrade sidekiq-cron from 0.4.4 to 0.6.0 in order to fix rufus-scheduler exception
mentioned in issue #31554 (closed)
mentioned in merge request !10976 (merged)
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
marked the checklist item Changelog entry added, if necessary as completed
marked the checklist item Conform by the merge request performance guides as completed
marked the checklist item Conform by the style guides as completed
@grzesiek Could you review, please?
This MR has just finished and the pipeline has passed. I also left a couple of explanations for each fix.- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
@dosuken123 Awesome work! I left couple of notes, this is almost ready!
assigned to @dosuken123
changed milestone to %9.1
added 110 commits
-
843cb592...d6dfbf02 - 101 commits from branch
gitlab-org:master
- c8585d16 - Fix lazy error handling of cron parser
- b3d3b51e - Add feature test, which reproduced 500
- f023f77a - Extract tzinfo from Rails timezone(Fix for servertimezone ailiases)
- e0460955 - Add changelog
- 376e19eb - Add spec for sidekiq-cron dependency
- ac2dac0f - Fix spec
- 7a7e199f - Upgrade sidekiq-cron from 0.4.4 to 0.6.0 in order to fix rufus-scheduler exception
- 65a660fc - Split MR
- ad5789f1 - Support ActiveSupport::TimeZone
Toggle commit list-
843cb592...d6dfbf02 - 101 commits from branch
- Resolved by Shinya Maeda
added 36 commits
-
ad5789f1...54beb93a - 25 commits from branch
gitlab-org:master
- b5a25bb0 - Fix lazy error handling of cron parser
- 0ad609e6 - Add feature test, which reproduced 500
- d6b331ac - Extract tzinfo from Rails timezone(Fix for servertimezone ailiases)
- e5ad686e - Add changelog
- ecc6c11b - Add spec for sidekiq-cron dependency
- 59d80f33 - Fix spec
- 435e19f4 - Upgrade sidekiq-cron from 0.4.4 to 0.6.0 in order to fix rufus-scheduler exception
- fe324063 - Split MR
- 177332f7 - Support ActiveSupport::TimeZone
- b3c7aff0 - Correct triggers_spec.rb
- 48fbd9fb - Correct cron_parser_spec.rb
Toggle commit list-
ad5789f1...54beb93a - 25 commits from branch
assigned to @grzesiek
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
@dosuken123 Almost done! I left just a few minor nitpicks.
assigned to @dosuken123
added 2 commits
added 83 commits
-
35e531a0...9fd1a35f - 70 commits from branch
gitlab-org:master
- 93e5553e - Fix lazy error handling of cron parser
- db6ba8e2 - Add feature test, which reproduced 500
- c336ceb4 - Extract tzinfo from Rails timezone(Fix for servertimezone ailiases)
- 2bbe75d7 - Add changelog
- 92f19ca2 - Add spec for sidekiq-cron dependency
- 214ba0d7 - Fix spec
- 3a0c91bf - Upgrade sidekiq-cron from 0.4.4 to 0.6.0 in order to fix rufus-scheduler exception
- 7135e118 - Split MR
- 941dfb6e - Support ActiveSupport::TimeZone
- dc2548dc - Correct triggers_spec.rb
- 66e13ba7 - Correct cron_parser_spec.rb
- da86d103 - Make changelog readable
- e8a8d20f - Fold long lines wisely
Toggle commit list-
35e531a0...9fd1a35f - 70 commits from branch
@grzesiek Could you review again? Pipeline passed! Thank you!
@dosuken123 Looks good! Let's pass to a maintainer.
assigned to @rymai
- Resolved by username-removed-128633
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
- Resolved by Shinya Maeda
@dosuken123 Thanks, nice work. I left a couple of notes.
assigned to @dosuken123
added 63 commits
-
e8a8d20f...185fd98f - 46 commits from branch
gitlab-org:master
- bd440954 - Fix lazy error handling of cron parser
- 0763ef1c - Add feature test, which reproduced 500
- effd9b6d - Extract tzinfo from Rails timezone(Fix for servertimezone ailiases)
- cb38930d - Add changelog
- 62ae9f30 - Add spec for sidekiq-cron dependency
- e84da1d7 - Fix spec
- 4de9bb5e - Upgrade sidekiq-cron from 0.4.4 to 0.6.0 in order to fix rufus-scheduler exception
- 6064857c - Split MR
- 8573fa9a - Support ActiveSupport::TimeZone
- 6f71b7ba - Correct triggers_spec.rb
- 32c3a8b8 - Correct cron_parser_spec.rb
- 852749c7 - Make changelog readable
- 73efef7c - Fold long lines wisely
- 6a213dc9 - Ommit it description if it is simple
- 277c533a - Revise commnets on try_parse_cron
- 608fab6f - Change Gitlab to GitLab
- 63cfb73c - Move spec to right hierarchy
Toggle commit list-
e8a8d20f...185fd98f - 46 commits from branch
@dosuken123 Thanks, looks good to me!
assigned to @rymai
mentioned in commit d88b9ac7
mentioned in commit 85d5ca37