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

Queue parameter is ignored when scheduling job with IBackgroundJobClient #799

Closed
shatl opened this Issue Jan 23, 2017 · 10 comments

Comments

5 participants
@shatl
Copy link

shatl commented Jan 23, 2017

Hi,
same job goes to different HF queues when enqueued using static method and IBackgroundJobClient

  // .NET Core controllerm _jobs injected in constructor 
 
        [HttpGet("{message}", Name = "Echo")]
        [Route("echo")]
        public string Echo(string message)
        {
            // goes to default queue
            /* IBackgroundJobClient*/ _jobs.Enqueue<ITestJob>(x => x.DoIt(message));
            // goes to correct queue
            BackgroundJob.Enqueue<ITestJob>(x => x.DoIt(message));
            return message;
        }

    public interface ITestJob
    {
        [Queue(Settings.HangfireTransmitQueueName)]
        void DoIt(string msg);
    }
    class TestJob : ITestJob
    {
        public void DoIt(string msg)
        {
            
        }
    }
@pieceofsummer

This comment has been minimized.

Copy link
Contributor

pieceofsummer commented Jan 24, 2017

Wow, looks like one of the bugs I've just fixed in #800 :)

The reason is, IBackgroundJobClient uses IJobFilterProvider service from IoC (which is GlobalJobFilters.Filters), while the static wrapper uses JobFilterProviders.Providers. The latter combines global job filters from the former one with filters coming from class/method attributes. Hence, IBackgroundJobClient just knows nothing about attributes on your method.

While waiting for the official update, you may try a quick fix – add the following line to ConfigureServices() right before AddHangfire():

services.AddSingleton<IJobFilterProvider>(_ => JobFilterProviders.Providers); // <-- !!!

services.AddHangfire(config => {
    ...
});
@shatl

This comment has been minimized.

Copy link
Author

shatl commented Jan 24, 2017

Awesome!
Thanks @pieceofsummer

@shatl

This comment has been minimized.

Copy link
Author

shatl commented Jan 26, 2017

Guys, any ETA on release date for this?

@shatl

This comment has been minimized.

Copy link
Author

shatl commented Feb 10, 2017

any news so far?

@madannes

This comment has been minimized.

Copy link

madannes commented Mar 6, 2017

I've just run across this same issue in v1.6.9 on .NET 4.6.2. In glancing at the commits in the pull request, it looks like you're only affecting the .NET Core code. Should a separate issue be created for .NET 4.6.2?

@pieceofsummer

This comment has been minimized.

Copy link
Contributor

pieceofsummer commented Mar 6, 2017

Yes, that was mostly the .NET Core issue.

I have really never used Hangfire on a classic .NET, so can't say for sure. But looking at Hangfire.AspNet package, I don't really see any code affecting the job filter providers.

So it might be just your code. Or maybe I'm looking at the wrong package :)

@burningice2866

This comment has been minimized.

Copy link
Contributor

burningice2866 commented Mar 6, 2017

@pieceofsummer the Hangfire.AspNet package only contains some helper-classes to ensure proper shutdown on asp.net - personally i've never used any of it even though i run a dozen of sites using HangFire on Asp.Net.

As far as i understand all of HangFire runs on Classic .Net of the box and only has special behavior for .Net Core.

@burningice2866

This comment has been minimized.

Copy link
Contributor

burningice2866 commented Mar 6, 2017

Personally i would prefer to see everything related to DI and Logging from the AspNetCore package be backported back to Core since neither Microsoft.Extensions.DependencyInjection or Microsoft.Extensions.Logging is restricted to be used only on .Net Core.

@shatl

This comment has been minimized.

Copy link
Author

shatl commented Jan 8, 2018

Any updates on this ?

@odinserj

This comment has been minimized.

Copy link
Member

odinserj commented Mar 30, 2018

Closing this issue as #800 already merged.

@odinserj odinserj closed this Mar 30, 2018

@odinserj odinserj referenced this issue Mar 30, 2018

Merged

Release 1.6.18 #1156

19 of 19 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment