Skip to content

Inherit job class attrs#18

Open
jingta wants to merge 6 commits intomasterfrom
inherit_job_class_attrs
Open

Inherit job class attrs#18
jingta wants to merge 6 commits intomasterfrom
inherit_job_class_attrs

Conversation

@jingta
Copy link
Contributor

@jingta jingta commented May 8, 2017

Currently Jobs didn't support multi level inheritance as jake was hoping they would. this fixes inherited attributes from a parent job class (EXCEPT tube name)

jingta and others added 2 commits May 5, 2017 18:50
Refactor attributes to fetch from self, the parent class, the config, in that order
@jingta jingta requested review from att14, tpabla and victoryftw May 8, 2017 22:20
Copy link
Contributor

@victoryftw victoryftw left a comment

Choose a reason for hiding this comment

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

This seems good to me. This functionality is pretty important in my mind, since wanting to separate production/test mode queues comes up a lot.

@victoryftw
Copy link
Contributor

Le refactor is up @att14 @jingta

Let me know if you have suggestions/feelings/cooking advice/things you want changed!

keys
end
end
def self.get_serializable_context_keys
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren't these instance methods?

@victoryftw
Copy link
Contributor

I changed a bunch of the attrs to use instance method where I thought it made sense (i.e. the ones that seemed job-scope configurable). We might be able to do the same for retry_attempts, but it seemed like the "job" that we actual dequeue in worker.rb isn't actually an instance of the job class. @jingta can you confirm that behavior or is it just that the tests are weird?

@Justintime50
Copy link
Member

Still needed?

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.

4 participants