Skip to content
This repository has been archived by the owner on Jul 3, 2024. It is now read-only.

Expose more about job using metadata #158

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

Conversation

roelvanduijnhoven
Copy link
Collaborator

This PR extends the DoctrineQueue to publish scheduled and priority. Previously only id got exposed to the outside world.

My use-case is that I would like to add a strategy that can send the time it took before the job started to an outside monitoring system.

@roelvanduijnhoven
Copy link
Collaborator Author

@basz What do you think about this PR?

@roelvanduijnhoven roelvanduijnhoven requested a review from basz May 25, 2020 09:58
Copy link
Collaborator

@basz basz left a comment

Choose a reason for hiding this comment

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

Looks good!

@basz
Copy link
Collaborator

basz commented May 25, 2020

We could -eventually- consider adding an interface exposing these additions? 🤷‍♂️

DoctrineJobInterface extends https://github.com/JouwWeb/SlmQueue/blob/master/src/Job/JobInterface.php

@roelvanduijnhoven
Copy link
Collaborator Author

@basz Mm. When I read your post that made sense. But.. now I am implementing it, it does not seem to work.

The JobManager returns job instances: so it is up to the user of this application to decide what is returned, and what interface that thing will adhere to. I do not think it is a good idea to force them to implement the DoctrineJobInterface.

I think the path I initially took is still our best bet. Although that does feel somewhat hacky ..

@basz
Copy link
Collaborator

basz commented Jun 4, 2020

Ok.

But,... We don't need to force users to use it. They may use an DoctrineJobInterface on their Jobs. The JobManager doesn't care... Still returns JobInterfaces. I agree it would mainly be for IDE code completion...

@roelvanduijnhoven
Copy link
Collaborator Author

@basz The way I am intending to use this feature is to track how long it took before a job gets executed, after it's scheduled date has passed. That will however only give a proper reading if all jobs implement this. And this fails to work if you import a job that depends on SlmQueue from another repo.

But I do get your point... What if we do both?

  • We always write the metadata.
  • We provide a DoctrineJobInterface with utility methods that read from metadata.

I think that is an enhancement to my original proposal. And... maybe also what you meant initially?

@basz
Copy link
Collaborator

basz commented Jun 4, 2020

sounds decent! 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants