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

Monitor queued jobs #131

Merged
merged 3 commits into from
Aug 25, 2023
Merged

Conversation

AsemAlalami
Copy link
Contributor

@AsemAlalami AsemAlalami commented Jul 13, 2023

Monitor queued jobs #62

also, I think it fixed the #76 issue

the PR adds "Initial Data" feature that initializes custom data to the monitor job

@AsemAlalami AsemAlalami changed the title monitor queued jobs Monitor queued jobs Jul 13, 2023
@romanzipp
Copy link
Owner

Nice work! Thanks!
What do you think about adding a config option which needs to be implicitly enabled to enable monitoring queued jobs?

@AsemAlalami
Copy link
Contributor Author

Yes, I can do that
But wonder why someone wouldn't want to monitor queued jobs?!

@romanzipp
Copy link
Owner

romanzipp commented Jul 20, 2023

I was thinking about how this could impact existing implementations which are coupled to the Monitor model. Anyways we will have to publish this change in a new major version so I'd still prefer a config option but we can leave it enabled by default.

@AsemAlalami
Copy link
Contributor Author

I added a config option, but please take a look to my comment

@romanzipp
Copy link
Owner

romanzipp commented Aug 25, 2023

Do you have anything to add or is this ready to merge?

@AsemAlalami
Copy link
Contributor Author

I think it's ready to merge

@romanzipp romanzipp merged commit 5025d57 into romanzipp:master Aug 25, 2023
@romanzipp
Copy link
Owner

romanzipp commented Sep 4, 2023

Just a FYI, there are tests failing with the upcoming version. I'm looking into it but if you have an idea feel free to file a PR!

#136

@romanzipp romanzipp mentioned this pull request Sep 4, 2023
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