Hangfire: DisplayName not supported in .NET Core?

[fortinmike]

I don't understand why DisplayName logic is excluded from .NET Core builds in HtmlHelper (using #if NETFULL). It seems to me like it should be possible to include that code; DisplayNameAttribute and reflection machinery are available in .NET Core.

I'm building an ASP.NET Core 1.0 app with Hangfire and would love having this. The dashboard is much less useful without that feature.

[pieceofsummer]

Adding System.ComponentModel.Primitives package solves missing DisplayNameAttribute issue. No other code changes are needed.

[odinserj]

The whole package, just for the one attribute... I've tried hard to keep as few dependencies as possible, that's why I temporarily removed it. This is not a huge problem, but there is also an alternative – write a simple attribute and put it to Hangfire, the problem is naming, that shouldn't intersect with ComponentModel's ones.

So I'm currently in doubts regarding the implementation.

[fortinmike]

To me this is indicative of the fact that the attribute should be part of Hangfire, not grabbed from ComponentModel. Especially if the dependency exists only to satisfy this feature. In fact, I was initially surprised to notice that the attribute was _not_ part of Hangfire.

Are the semantics of ComponentModel's DisplayNameAttribute correct for this use case? Even if so, I think a Hangfire-provided attribute would be best. Bringing that back into Hangfire would also allow extending the attribute to package additional information in the future.

(As a side-note, I find it cumbersome that Hangfire relies a lot on attributes, especially for this and other advanced scenarios like starting tasks on specific queues programmatically, etc. Still, great work! The library is super-useful to us.)

[odinserj]

To me this is indicative of the fact that the attribute should be part of Hangfire, not grabbed from ComponentModel. Especially if the dependency exists only to satisfy this feature. In fact, I was initially surprised to notice that the attribute was not part of Hangfire.

The attribute is used by ASP.NET MVC in models, and in some other things I can't remember now. Since it was a part of .NET Framework at that moment, I decided to reuse it, instead of adding yet another class. But for .NET Core indeed, it is better to create a new one.

So, let's schedule it to 1.7.0.

I find it cumbersome that Hangfire relies a lot on attributes, especially for this and other advanced scenarios like starting tasks on specific queues programmatically

So attributes are good or bad? Seems like I don't understand :smiley:

[fortinmike]

Good, I'm looking forward to this enhancement. Thanks!

So attributes are good or bad? Seems like I don't understand 😃

I meant that, to me, attributes are a hindrance in those scenarios, mostly because the configuration information they carry can't be provided at runtime without jumping through hoops (e.g. working with Hangfire's innards directly, etc., which is less documented).

For example, it would be nice to provide a context-relevant description when enqueueing a task, rather than having to pass that information as an additional parameter to the task's method and using DisplayNameAttribute to extract it. It would also be nice to be able to specify a queue to run a task on at enqueue time (in BackgroundJob.Enqueue).

In essence, I would personally expect an API to provide easy, programmatic ways to configure things first, then to provide attributes as a convenience. Going attributes-first complexifies advanced scenarios IMHO. That's what I meant. Though I understand changing this could imply much refactoring and potentially API-breaking changes. Just a thought.

[barclayadam]

About a year ago I had opened a PR that would support the setting of a DisplayName programatically at the creation time of a job @ https://github.com/HangfireIO/Hangfire/pull/413

It's certainly something that I had found a need for, as the at-compile-time nature of attributes did not provide us with the necessary hooks to get what we wanted.

In the end I conceded the design a little and just overrode the ToString method of a base class (which exists purely for this purpose). Not great, but it did mean I no longer had to maintain a separate fork of the codebase

[tonidy]

In essence, I would personally expect an API to provide easy, programmatic ways to configure things first, then to provide attributes as a convenience. Going attributes-first complexifies advanced scenarios IMHO. That's what I meant. Though I understand changing this could imply much refactoring and potentially API-breaking changes. Just a thought.

Agreed with @fortinmike, as attribute runs in compile time, IMHO this good only for simple scenario. For complex scenario we need something that we can change at run time.

[pieceofsummer]

There's also a relevant PR #660 out there, but it is still up to @odinserj to decide.

[gregbty]

Any workaround for this (other than forking)? I tried wrapping the Job class but it had no effect because of how it's pulled for the dashboard.

[graemechristie]

I think adding a new Attribute to in one of the Hangfire assemblies, specifically for job naming would not be the best path. This would mean that any assembley that that contains a method that needs to be named in the dashboard would need to take a dependency on the the Hangfire libs. This would be ok for large, monolithic applications, but complicates the dependency graph for more compositional apps.

Ideally, a type that is serialised into a hangfire database, should be able to be blissfully unaware that is being used by hangfire - and this is already generally the case - other than with the requirement for using an attribute for the Job naming.

I think @pieceofsummer 's approach in #660 is generally the correct direction to take. This gives us an extensibility point to provide whatever approach is required. The attribute based method can be left in place as the default for Full Framework, without any breaking changes. .NET Core users, and FF users who want to opt in, can use the new implementation (and a custom attribute based approach could be supplied as a separate nuget package if anyone required this)

If @pieceofsummer is not available, I am happy to rebase/take ownership #660 if @odinserj agrees this is a good way forward.

[pieceofsummer]

Closing as #660 and #993 are merged now