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

Add basic cron based 'timer' mode #88

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

tom--pollard
Copy link
Contributor

@tom--pollard tom--pollard commented Apr 15, 2021

This leverages the cutoff_delta config, adding the option to set an internal cron like scheduler for triggering the polling of feeds. The current implementation somewhat mimics the current trigger which is driven by requests from an external scheduled service.

As mentioned in #85 this is one approach that uses a static frequency, this could be expanded to be dynamic (storing an internal value for time_of_last_poll, to use for the cuttoff threshold instead potentially on a per feed basis). Further to that is the consideration of if a httpserver should even be launched in this standalone mode, or even a hybrid approach. These can all be considered in followup PRs

@tom--pollard tom--pollard force-pushed the internal_scheduler branch 2 times, most recently from 29af94e to 3f756ac Compare April 15, 2021 11:30
Copy link
Member

@naveensrinivasan naveensrinivasan left a comment

Choose a reason for hiding this comment

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

👍

cmd/scheduled-feed/main.go Outdated Show resolved Hide resolved
cmd/scheduled-feed/main.go Outdated Show resolved Hide resolved
cmd/scheduled-feed/main.go Show resolved Hide resolved
Copy link
Contributor

@Qinusty Qinusty left a comment

Choose a reason for hiding this comment

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

Mostly looks good to me, minor comment.

cmd/scheduled-feed/main.go Outdated Show resolved Hide resolved
Copy link
Contributor

@Qinusty Qinusty left a comment

Choose a reason for hiding this comment

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

LGTM!

@tom--pollard tom--pollard merged commit c554462 into ossf:main Apr 22, 2021
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.

3 participants