Skip to content

LG-15803: Include all arguments in A/B report base job class initialization#11913

Merged
aduth merged 1 commit intomainfrom
aduth-ab-test-job-arguments
Feb 24, 2025
Merged

LG-15803: Include all arguments in A/B report base job class initialization#11913
aduth merged 1 commit intomainfrom
aduth-ab-test-job-arguments

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Feb 24, 2025

🎫 Ticket

LG-15803

🛠 Summary of changes

Fixes an error where the recurring job for A/B test reporting doesn't complete because the perform method doesn't receive all arguments.

The underlying issue is that the super call did not pass all of the original position arguments received by the call, so the job did not have the arguments to send to the enqueued perform (ref).

📜 Testing Plan

rspec spec/jobs/reports/ab_tests_report_spec.rb

You could also locally modify the job configuration to either use a shorter cron value, or modify the cron_24h value to occur on a quicker cadence, and then ensure that the job does not error when run via bundle exec good_job start.

@aduth aduth requested a review from a team February 24, 2025 19:50
Copy link
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

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

LGTM! 👍
Great find.

@aduth
Copy link
Contributor Author

aduth commented Feb 24, 2025

You could also locally modify the job configuration to either use a shorter cron value, or modify the cron_24h value to occur on a quicker cadence, and then ensure that the job does not error when run via bundle exec good_job start.

Testing this locally, it's a little harder to verify than I anticipated, because the error isn't output to the terminal. But you can find (or not find, after these changes) the exception_class value in log/workers.log.

Something like:

tail -n0 -F log/workers.log | jq .

@aduth aduth merged commit ad5524d into main Feb 24, 2025
2 checks passed
@aduth aduth deleted the aduth-ab-test-job-arguments branch February 24, 2025 20:53
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.

2 participants