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

Basic CloudScheduler implementation #71

Closed
wants to merge 3 commits into from

Conversation

ceritium
Copy link

Hi, based on this issue #20, I implemented a pretty basic cloud scheduler backend for period jobs.

I am testing it on a personal project. I trigger the "synchronize!" method with a rake task from cloud build, but I don't have experience deploying apps on Google Cloud and Google Cloud Run, so I am unsure if it is the best approach.

My rake task looks like this:

# app.rake

namespace :app do
  task setup_cloud_scheduler: :environment do
    Cloudtasker::CloudScheduler::Manager.synchronize!('config/cloudtasker_cron.yml')
  end
end

And the config file:

# config/cloudtasker_cron.yml

check_feeds_test:
  worker: 'CheckSubscriptionsJob'
  cron: '0 * * * *'

Of course, the PR needs some specs and some refactoring, but I would like your opinion about the general approach before investing time in working on the details.

Many thanks!

@alachaum
Copy link
Member

Hey @ceritium - sorry for the late reply. The last few months have been a bit hectic on my side.

I think the general approach looks good. The concept is to 1) have a rake task that synchronizes jobs with GCP Scheduler under a specific namespace 2) configure the job payloads so that they are linked back to a specific worker class and 3) make sure that the controller/handler is able to receive and process jobs coming from GCP Scheduler.

Have you been able to try step (3)? Is it working as is?

@ceritium
Copy link
Author

Hi @alachaum, yes, that's the idea, and it's working as you described. If you like the approach, I can improve the PR, including a rake task and specs. Please let me know if you have any suggestions about naming the classes and methods.

@alachaum
Copy link
Member

The approach and naming look good to me 👍 Worst case it's the kind of thing I can tweak during my final testing after merging into master.

If you do further work on this PR, can you also add method/class documentation? It makes it easier for people to get on board 😉

@ceritium
Copy link
Author

I will not continue with the PR because I have not been using it actively for some time and haven't had much spare time lately. The PR needs specs coverage, comments, and maybe some refactoring; however, it works fine for me on production. Please, feel free to reuse it if you find it useful.

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.

2 participants