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

Added emission of kernel events for GearmanJob lifecycle #188

Open
wants to merge 1 commit into
base: 3
Choose a base branch
from

Conversation

Eloar
Copy link

@Eloar Eloar commented Jan 19, 2018

  • added GearmanJobWrapper to support GearmanJob event emission
  • documentation enhanced

- added GearmanJobWrapper to support GearmanJob event emition
- documentation enhanced
@Eloar Eloar changed the title - added emission of kernel events for GearmanJob lifecycle Added emission of kernel events for GearmanJob lifecycle Jan 19, 2018
@bmeynell
Copy link
Contributor

Is there an issue number or link to discussion anywhere prompting this PR? Curious about background/context.

@Eloar
Copy link
Author

Eloar commented Jan 31, 2018

Hmm, I think there is none. I made this change so I could easier follow lifecycle of each job passed to worker. Based on that I could make listener storing progress of each task to MongoDB for any client to follow. Second use was to clear EntityManager before each job was started.

@bmeynell
Copy link
Contributor

bmeynell commented Jan 31, 2018

@Eloar - I see you included some documentation in your PR. Could you leverage that to create a GH issue and then associate this PR to that issue? My knee-jerk reaction was that we already have events that can be subscribed to so how is this PR different than what we currently have?

In general having an associated GH issue + PR that references it (instead of a single PR), as a rule of thumb, makes new users easier follow the progression of how changes came to be over time as well as overall context for the change. @daum - agree?

If cannot do, maybe I can squeeze in some time to create a GH based on the docs you provided.

@daum
Copy link
Collaborator

daum commented Jan 31, 2018

Yeah it would help to understand a bit more via an issue and why the existing kernel events didn't work in your case.

@Eloar
Copy link
Author

Eloar commented Feb 9, 2018

I looked through issues list and my PR relates to #99. I'm using backgrounds jobs for all my workers. Async clients gets no information about job lifecycle (kind of obvious but poorly documented by Gearman itself). With my change I was able to emit events worker side and notify client different way in listeners.

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