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

Feature Request: Extract data from job to use as job Title #349

Closed
bugs181 opened this issue Dec 4, 2021 · 20 comments · Fixed by #359
Closed

Feature Request: Extract data from job to use as job Title #349

bugs181 opened this issue Dec 4, 2021 · 20 comments · Fixed by #359

Comments

@bugs181
Copy link

bugs181 commented Dec 4, 2021

A few versions back, I was "abusing" the named job parameter to have better titles on the dashboard. After upgrading to the latest; when named jobs are used we get the lovely "Missing process handler" error. I'll try working on a PR to add support to make this a little more generic which can be useful for other cases.

Original named jobs method that I was abusing to accomplish this goal:

await bullQueue.add(jobItem.title, jobItem)

Proposal 1:

Append title parameter to QueueOptions, which is just a generic JSON extractor.

Example:

const bullQueue = new Bull('job-queue', { title: "jobTitle" })

...

const jobItem = {
  jobTitle: item.page.title,
  url: item.page.href,
}

await bullQueue.add(jobItem)

Proposal 2:

Append jobTitle? parameter to JobOptions, which is just a generic JSON extractor.

Example:

const bullQueue = new Bull('job-queue')

...

const jobItem = {
  jobTitle: item.page.title,
  url: item.page.href,
}

await bullQueue.add(jobItem, { jobTitle: "jobTitle" })

Expected Output:

Expected title output would be something along the lines of:

`${jobName} - ${jobTitle}`

Bonus options:

  • replace?: boolean | undefined; - Replace entire job title for the expected output section, instead of appending jobTitle to jobName.
@felixmosh
Copy link
Owner

Did you tried options.prefix ? I think that it's exactly what you need :]

@bugs181
Copy link
Author

bugs181 commented Dec 4, 2021

Did you tried options.prefix ? I think that it's exactly what you need :]

Perhaps my wording wasn't clear or I am misunderstanding the use of prefix. I need the job titles to be unique per job. I was under the impression, the prefix is just static and used for all jobs. Am I incorrect?

@felixmosh
Copy link
Owner

Yes, you are correct.

Can you explain what do you want?

@bugs181
Copy link
Author

bugs181 commented Dec 7, 2021

The easiest explanation would be something along the lines of this:

await bullQueue.add(jobItem, { jobTitle: "Downloading 123" })
await bullQueue.add(jobItem, { jobTitle: "Unzipping 123" })
await bullQueue.add(jobItem, { jobTitle: "Installing 123" })

I understand that example may still be confusing due to the above example should use different queues - BUT it's just an example. I have a case where all jobs are the same type, but I want the titles to be unique at the time of adding them.

@felixmosh
Copy link
Owner

@bugs181 , I think that you've got confused, bull-board is a dashboard for your queues, It currently doesn't allows to add jobs to the queue, it only reflects it's data.

If you are using bull you can create a job with a name (which will be reflected on the dashboard).

if you are using bullmq you can create a job with a name like so: https://docs.bullmq.io/guide/jobs/job-ids.

@bugs181
Copy link
Author

bugs181 commented Dec 8, 2021

@bugs181 , I think that you've got confused, bull-board is a dashboard for your queues, It currently doesn't allows to add jobs to the queue, it only reflects it's data.

If you are using bull you can create a job with a name (which will be reflected on the dashboard).

if you are using bullmq you can create a job with a name like so: https://docs.bullmq.io/guide/jobs/job-ids.

Unfortunately, I think there's a language barrier here. I'm fully aware of what bull-dashboard is. Displaying job data is exactly the purpose of the bull-board. Named jobs do not fulfill this purpose. I believe that a more generic approach, allowing users to dive into the job data to retrieve specific data for their job title on bull dashboard is important.

Pictures are worth a thousand words, so let me show you what I'm trying to accomplish instead.

bull-board

The reason I was using bull to show this example, is because I was trying to make clear what expectations we could have about createBullBoard options. Setting the title props could be done on the BullAdapter constructor itself, or somewhere else. I haven't yet looked at the source so was only providing proposals of how bull and dashboard could be combined.

The other reason I used bull examples, was to illustrate how I used to accomplish this goal in the past. Hopefully this helps clarify my intentions.

This also brings into focus the fact that if we don't use named jobs we get __default__ as the job title (as can be seen in the screenshot)


TL;DR

For clarification: The whole point of this feature request is to ask for the ability to extract data from the job data, and display that as the title.

@bugs181 bugs181 changed the title Feature Request: Better job title support Feature Request: Extract data from job to use as job Title Dec 8, 2021
@felixmosh
Copy link
Owner

Ok, I think that i get it, what is your use case for this feature?

@bugs181
Copy link
Author

bugs181 commented Dec 8, 2021

Many more use cases than the one example above. Running a custom getTitle function would make this feature request more generic so it fits all my use cases and then some.

By using an option we can detect this option in queues->formatJob and override the name property.

A proposal example could be useful here:

const queueOptions = {
  getTitle: function(jobProps) {
    return jobProps.data.title ? jobProps.data.title : jobProps.name
  }
}

createBullBoard({
  queues: [new BullAdapter(bullQueue, queueOptions)],
  serverAdapter,
})

Example titles could be different job data exposed for staged environments, job paths that are taken to arrive at the queue, etc. Basically, any kind of data that could be useful at a glance.

One such use-case I have is exposing Drone CI repo names.

This would make my titles look like these:

  • Cloning repo bugs181/ML-Target
  • Building repo bugs181/ML-Target
  • Deploying repo bugs181/ML-Target

Current solution:

Currently, I'm overriding formatJob on line 60 of queues.js with:

name: jobProps.data.title ? jobProps.data.title : jobProps.name,

This would essentially get turned into something along the lines of this (with my above proposal):

const queueOptions = {
  getTitle: function(jobProps) {
    const repo = jobProps.data
    return repo.action && repo.name ? `${repo.action} repo ${repo.name}` : jobProps.name
  }
}

createBullBoard({
  queues: [new BullAdapter(bullQueue, queueOptions)],
  serverAdapter,
})

// Then in my bull I use this:
const jobItem = {
  task: 'clone',
  action: 'Cloning',
  name: 'bugs181/bull-board-example'
}

await bullQueue.add(jobItem)

This example would output Cloning repo bugs181/bull-board-example

@felixmosh
Copy link
Owner

We do have a built in formatters pattern that applies only on data & returnValue job props.

If I will add a support for formatting the name prop, will it help your use-case?

So, you will able to pass a formatter for the name field like so:

// server.js

const Queue = require('bull')
const QueueMQ = require('bullmq')
const { setQueues, BullMQAdapter, BullAdapter } = require('bull-board')

const someQueue = new Queue()
const someOtherQueue = new Queue()
const queueMQ = new QueueMQ()

const someQueueAdapter = new BullAdapter(someQueue);

// This is the proposed api
someQueueAdapter.setFormatter('name', function(jobProps) {
    const repo = jobProps.data
    return repo.action && repo.name ? `${repo.action} repo ${repo.name}` : jobProps.name
  }); // this will apply name formatter

setQueues([
  someQueueAdapter,
  new BullAdapter(someOtherQueue),
  new BullMQAdapter(queueMQ),
]);

WDYT?

bugs181 added a commit to bugs181/bull-board that referenced this issue Dec 8, 2021
@bugs181
Copy link
Author

bugs181 commented Dec 8, 2021

Your proposed solution could work. Not sure what other implications there are of formatting the data, though. It might be tricky to evaluate where the data came from if it modifies the data object in the dashboard, however. With my proposed solution, and PR #358 - we know exactly how the formatting happens and that it's only purpose is for display.

@felixmosh
Copy link
Owner

I don't agree, we are already have the concept of formatting other fields on the job object, name is one of them.
And it is better to follow the same convention instead of creating a new one.

@bugs181
Copy link
Author

bugs181 commented Dec 8, 2021

I think I see one more issue with someQueueAdapter. Will the formatter be applied to all queues or just the one? The reasoning this couldn't work for my use-case is when I have a dashboard where titles are very specific to the queue flow.

Edit: actually, I was misreading the code. It appears you are just setting the function to setFormatter. I feel like this is "unclean" compared to passing the option in though.

@felixmosh
Copy link
Owner

  1. Formatters are queue based
  2. Formatters are existing pattern
  3. It is a matter of code style, you don't like mine and I don't think that yours is much better :]

If it solves your issue, I will make a PR, if you want to help, It will be faster, but I do want to continue the same existing pattern.

@bugs181
Copy link
Author

bugs181 commented Dec 8, 2021

That's acceptable. Code style aside, to each their own. No complaints here either way as long as we can accomplish the same task. I had a look at setFormatter but couldn't see how this would be applied to the name property. I'm largely unfamiliar with the code base right now. I'll keep taking a swing at it; but I have doubts I'll be able to make it work with my limited knowledge of the code.

@felixmosh
Copy link
Owner

No problem, I will make a PR. :]

@bugs181
Copy link
Author

bugs181 commented Dec 8, 2021

Doh! Silly me. I was over-complicating it.

This should do the trick:
name: queue.format('name', jobProps),

Unless you see something I missed, I'll revert the PR and apply this patch.

@felixmosh
Copy link
Owner

felixmosh commented Dec 8, 2021

Doh! Silly me. I was over-complicating it.

This should do the trick: name: queue.format('name', jobProps),

Unless you see something I missed, I'll revert the PR and apply this patch.

Yeap :]

@bugs181
Copy link
Author

bugs181 commented Dec 8, 2021

Thanks so much for your help! This looks great!

@felixmosh
Copy link
Owner

Thank you for you patience

@felixmosh
Copy link
Owner

Released in v3.8.0

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 a pull request may close this issue.

2 participants