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.

Sign up for GitHub

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

Log payloads that are skipped becuse no jobs were run #42

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Log payloads that are skipped becuse no jobs were run #42

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a7a36c
Log payloads that are skipped becuse no jobs were run
iloveitaly May 19, 2016
1fb090a
Log payload state and job class when job is skipped
iloveitaly Jun 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion app/interactors/cangaroo/perform_jobs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module Cangaroo
class PerformJobs
include Cangaroo::Log
include Interactor

def call
Expand All @@ -15,7 +16,20 @@ def data
end

def enqueue_jobs(type, payload)
initialize_jobs(type, payload).select(&:perform?).each(&:enqueue)
initialize_jobs(type, payload).each do |job|
if job.perform?
# TODO log simplified info about payload info is being queued
Copy link

Choose a reason for hiding this comment

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

Annotation keywords like TODO should be all upper case, followed by a colon, and a space, then a note describing the problem.

job.enqueue
else
# TODO persist translation for audit trail?
Copy link

Choose a reason for hiding this comment

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

Annotation keywords like TODO should be all upper case, followed by a colon, and a space, then a note describing the problem.

Copy link
Member

Choose a reason for hiding this comment

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

@iloveitaly Can you explain how do you plan to use translation on the logging class?

Copy link
Contributor Author

@iloveitaly iloveitaly Sep 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This is referring to the Translation record on this PR #19

If the job is not queued it won't be saved to the translation log. However, I think it would be helpful to have a record of the translation even if the payload is not pushed to a job

Copy link
Member

Choose a reason for hiding this comment

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

@iloveitaly can you also remove the TODO comment please


log.info 'skipping job for payload',
skipped_job: job.class.to_s,
payload: payload,
Copy link

Choose a reason for hiding this comment

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

Align the parameters of a method call if they span more than one line.

payload_type: type,
payload_state: job.payload_state
Copy link
Member

Choose a reason for hiding this comment

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

@iloveitaly What's that 😸 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops! Forgot to remove this. This relies on the Translation class

https://github.com/nebulab/cangaroo/pull/19/files#diff-45c84b17b9d53ae7c13d03d296304e38R27

Copy link
Member

Choose a reason for hiding this comment

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

@iloveitaly can you please remove it, we'll add it in the future.

end
end
end

def initialize_jobs(type, payload)
Expand Down