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

Unwrap ActiveJob job class in WebUI #460

Merged
merged 8 commits into from
Dec 20, 2023

Conversation

ibrahima
Copy link
Contributor

@ibrahima ibrahima commented Dec 20, 2023

Unwraps ActiveJob job classes in the web UI.

This does not unwrap the arguments, but that would be a nice addition as well.

Based on similar logic from Sidekiq, thanks for the pointer @mperham! I barely understand Go so I hope the code is okay.

https://github.com/sidekiq/sidekiq/blob/73c150d0430a8394cadb5cd49218895b113613a0/lib/sidekiq/api.rb#L374-L389

Before:

image

After:

(This is from the "queue" page, which uses Type for the column, and apparently a darker font.)

image

image

Fixes #459

webui/helpers_test.go Outdated Show resolved Hide resolved
I got this by calling json.Marshal on a job that was rendered in the
Web UI.
The Rails 5 case is probably not exactly correct, but the important
thing for now is that it uses the correct wrapped job class.
webui/helpers_test.go Outdated Show resolved Hide resolved
webui/helpers_test.go Outdated Show resolved Hide resolved
@@ -415,6 +415,32 @@ func sortedLocaleNames(req *http.Request, fn func(string, bool)) {
}
}

func displayJobType(j *client.Job) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is based on https://github.com/sidekiq/sidekiq/blob/73c150d0430a8394cadb5cd49218895b113613a0/lib/sidekiq/api.rb#L374-L389. It handles regular ActiveJobs and also ActionMailer wrapped jobs, for Rails 5 and 6+.

I hope the error handling is reasonable, I think I got everything. I think that if it fails to get more specific info, it will fall back to the more general data. (E.g. if it can't get the mailer class info, it should revert to the job class, or the raw job.Type if nothing else fits.)

I do love how concise the Ruby is but it is kind of nice that the Go version forces you to handle possible errors if the data isn't in the shape that you expected. The nesting in Go seems kinda wild, not sure if there was a better way to write this.

}

func makeActiveJob(jobType string) *client.Job {
var payload = []byte(fmt.Sprintf(`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got these payloads by sticking this in my displayJobType function:

	out, err := json.Marshal(j)
	if err == nil {
		util.Infof("Job: %s", out)
	}

So this should be exactly what the Web UI receives at render time. I parametrized certain things as made sense. I left the IDs and timestamps and whatnot alone since there's nothing particularly interesting in those.


func makeActionMailerJob(jobType string, mailerClass string, mailerMethod string) *client.Job {
var payload = []byte(fmt.Sprintf(`
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the Job payload from a Rails 7.1 app. It seems to match the class in https://github.com/sidekiq/sidekiq/blob/73c150d0430a8394cadb5cd49218895b113613a0/test/api_test.rb#L34C135-L34C135, so I labeled it as Rails 6.x further down. I don't know if the specific fields are exactly the same as they would be in Rails 6, but for the purposes of getting the job type it doesn't matter.

expected: "FooJob",
},
{
name: "Rails 5.x mailer",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that in https://github.com/sidekiq/sidekiq/blob/73c150d0430a8394cadb5cd49218895b113613a0/test/api_test.rb#L30 the Rails 5 data is a bit smaller. Though I don't know if that's just because it doesn't include a serialized User object. In any case, for the purposes of this test the only difference is the job class, so I felt it was fine to use the same payload with different job classes for testing Rails 5 vs 6 handling.

@ibrahima ibrahima marked this pull request as ready for review December 20, 2023 22:25
@ibrahima ibrahima requested a review from mperham December 20, 2023 22:25
@mperham
Copy link
Collaborator

mperham commented Dec 20, 2023

Wow, this is great stuff. Well done, I'm impressed.

@mperham mperham merged commit b399b9f into contribsys:main Dec 20, 2023
1 check passed
@ibrahima ibrahima deleted the unwrap-activejob-class-webui branch December 20, 2023 23:55
@ibrahima
Copy link
Contributor Author

Thanks!

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 this pull request may close these issues.

Feature suggestion: Unwrap ActiveJob wrapped Job names in the WebUI
2 participants