-
Notifications
You must be signed in to change notification settings - Fork 3
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
support starting a reconciliation task with cron schedule #87
Conversation
Signed-off-by: Amir Malka <[email protected]>
Signed-off-by: Amir Malka <[email protected]>
Signed-off-by: Amir Malka <[email protected]>
Summary:
|
I am a bit concerned by the old package... maybe we should consider https://github.com/go-co-op/gocron instead |
adapters/backend/v1/adapter.go
Outdated
adapter: a, | ||
} | ||
|
||
if cfg.CronSchedule != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would instantiate another ticker instead of rewriting a new branch of code.
Please take a look at https://github.com/krayzpipes/cronticker/blob/main/cronticker/ticker.go (maybe we can just copy/paste this ticker and use the other cron package for our needs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amirmalka PTAL at my proposal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we release?
Signed-off-by: Matthias Bertschy <[email protected]>
Summary:
|
Overview