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

Sidekiq 6.4.0 changes behavior of Sidekiq::JobRecord#display_args #5133

Closed
thachck opened this issue Jan 22, 2022 · 2 comments · Fixed by #5134
Closed

Sidekiq 6.4.0 changes behavior of Sidekiq::JobRecord#display_args #5133

thachck opened this issue Jan 22, 2022 · 2 comments · Fixed by #5134

Comments

@thachck
Copy link
Contributor

thachck commented Jan 22, 2022

Ruby version: 2.7.5
Rails version: 6.1.4.4
Sidekiq version: 6.4.0

There is a change in behavior of Sidekiq::JobRecord#display_args in Sidekiq 6.4.0 compared with Sidekiq 6.3.1

Sidekiq 6.3.1
Screen Shot 2022-01-22 at 3 49 52 PM

Sidekiq 6.4.0
Screen Shot 2022-01-22 at 3 50 38 PM

The codebase is the same, it is just the Sidekiq version changes between the two screenshots.

Here is the shorten code snippet of the ZoomVideo::NotificationReminderService

class ZoomVideo
  class NotificationReminderService
    def initialize(zoom_video_id:)
    end

   def send_reminder_notification
   end
   
   def self.send_reminder_notification(*args)
      new(*args).send_reminder_notification
    end
  end
end

I guess this is related to #5120 where we add the support for keyword arguments in extensions, and we might need another method such as Sidekiq::Job#display_kwargs to handle this situation.

Please let me know if you think that is reasonable. I'm willing to add a PR for that.

Thanks for your great gem.

@mperham
Copy link
Collaborator

mperham commented Jan 22, 2022

Happy to get a PR for this.

@thachck
Copy link
Contributor Author

thachck commented Jan 23, 2022

@mperham Thanks. I have opened PR #5134 . Please have a look if you have time.

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