Rename Sidekiq::Worker to Sidekiq::Job

[mperham]

The term worker within the Sidekiq ecosystem is a net negative. Replace instances in the wiki and codebase with more precise terminology, e.g.

Sidekiq::Worker -> Sidekiq::Job
worker -> process, thread, job depending on context

[seuros]

This is going to be a major breaking change. Most addons of Sidekiq will need to be updated to work with the next version. Can you explain what the net negative in the term 'worker' ?

[mperham]

I know, the plan is to have a very long backward compatibility period. Ideally we make migration as simple as search/replace "Sidekiq::Worker" -> "Sidekiq::Job". My initial thought is:

• For 6.3.0, S::Worker will be an alias for S::Job
• For 6.4.0 it will print a deprecation warning
• For 7.0.0 it will break.

Restoring compatibility is as simple as Sidekiq::Worker = Sidekiq::Job in your initializer. It's quite possible 7.0 is still a year or more away since there's no major unreleased changes on the main branch.

"worker" is ambiguous, people use it to refer to processes ("I started a new worker"), types of jobs, executing threads ("I've got 10 workers"), jobs ("I enqueued 10 workers"), etc. It's a mess.

Does that make more sense?

[adamniedzielski]

@mperham

For 6.4.0 it will print a deprecation warning

I don't think that this happened 🤔 Are you still planning to deprecate in 6.x and remove in 7.0?

[mperham]

The only major issue I've seen so far is that ActiveJob owns the app/jobs directory whereas Sidekiq owns app/workers. I don't have a solution for that but I'm not sure it's a major issue: is it strange to see app/workers/hard_job.rb?

[jrochkind]

Rails will let you make whatever subdirectories of ./app you want, and they're all on the load path equivalently (with the name of the directory not actually mattering for loading), so people could also choose to keep them in ./app/sidekiq_jobs, or wherever they want. Just making the directory and putting the source there should work, no other config needed, sidekiq doesn't need to know about it.

[seuros]

Hmm, I think this change will cause a major refactoring effort , a lot of gems reference to that module.
For example, it will render ActiveJob incompatible with Sidekiq 7 unless we monkey patch it .

For folder naming, if i had to not use app/workers and since we agreed that app/jobs if for ActiveJob wrappers, i will use app/async_tasks or app/sidekiq_tasks.

[mperham]

I appreciate this feedback, you always have good advice.

I do want to create and document Sidekiq::Job. You are right that we can't realistically drop Sidekiq::Worker anytime soon, possibly forever. I will make the changes to use Sidekiq::Job everywhere but I will keep Sidekiq::Worker as a supported public API for the foreseeable future.

[mperham]

Plan is to release 6.2.3 tomorrow without Sidekiq::Job. 6.3.0 will contain Sidekiq::Job, which will simply be an alias for Sidekiq::Worker.

[seuros]

The alias is not a breaking change, so we could start testing it from 6.2.3 unofficially. Officially it will be 6.3.0. wdyt ?

[mperham]

The issue is that the Sidekiq API currently has a helper class called Sidekiq::Job so I had to refactor the API a bit. That refactor is a breaking change for any scripts that referenced that class. It's not a major or documented API but it's still breaking so I thought it better to introduce it in a minor version bump, not a patch.

[mperham]

Maybe it's probably better to introduce the breaking change now so people can fix their scripts and then introduce the new Sidekiq::Job alias in a future release?

[mperham]

Latest plan:

• Refactor API to remove Sidekiq::Job class name [6.2.x]
• Introduce Sidekiq::Job as opt-in alias (need to require it) [6.2.x]
• Make Sidekiq::Job a built-in alias [6.3]
• Refactor all "worker" verbage in codebase and wiki to use more specific term [6.3+]

[seuros]

@mperham sorry to comment here , but can you enable Discussions in this repo https://github.com/mperham/sidekiq/settings.

We could use it instead of have issues opened.

[mperham]

Sure, we can try it out.

[doits]

Looks like v6.2.3 broke https://github.com/kenaniah/sidekiq-status, because I get uninitialized constant Sidekiq::Job errors.

Afaik a manual require "sidekiq/job" is needed now to fix it, am I correct?

Shouldn't it be a non breaking change for a minor release and maybe break on v6.3.0?

Edit: Or it got renamed to JobRecord? So the upcoming new Sidekiq::Job has nothing to do with the old Sidekiq::Job?

[mperham]

Refactoring can change undocumented internal classes, even in patch releases. Sidekiq::Job was an undocumented API class, it was renamed to Sidekiq::JobRecord.

[mperham]

So the upcoming new Sidekiq::Job has nothing to do with the old Sidekiq::Job?

Correct, we removed Sidekiq::Job in this release so we can introduce a new module with that name in 6.3. Effectively we want to break everyone using it today because in 6.3.0, Sidekiq::Job will be completely different.

[doits]

Allright, thanks for the info.

[saltvedt]

This should have come with a bigger warning!

We started seeing NameError: uninitialized constant Sidekiq::Job in production after upgrading from 6.2.1 to 6.2.2 when running perform_later on RoR ApplicationJob which include Sidekiq::Status::Worker.

[vanboom]

Suggest that this change should increment the minor version. We just attempted to update from 6.2.0 to 6.2.2 - this is not a simple backward compatible "bug fix".

[seuros]

@mperham should we add a post install message ? I can submit a PR

[mperham]

@seuros no, I don't think it's necessary.