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

fix(unique-jobs): Unique jobs were not enqueuing jobs with keyword args #2857

Closed
wants to merge 1 commit into from

Conversation

ivannovosad
Copy link
Contributor

Context

There is a activejob-uniqueness bug that does not enqueue integration jobs when using named arguments.
This PR fixes that.

@ivannovosad ivannovosad force-pushed the fix-unique branch 3 times, most recently from 40b78af to 27ca595 Compare November 25, 2024 11:10
def perform(charge:, event:, billing_at: nil)
unique :until_executed, on_conflict: :log

def perform(charge, event, billing_at = nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

@ivannovosad why is this needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

(the removal of keyword arguments that is)

Copy link
Collaborator

@vincent-pochet vincent-pochet left a comment

Choose a reason for hiding this comment

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

What are we trying to solve here?

veeqo/activejob-uniqueness#75 does not mention keyword arguments, could you elaborate a bit on what is the issue?

Also, pay in advance billing job is no more unique in production? This feels a bit dangerous

@ivannovosad
Copy link
Contributor Author

The issue was that jobs were not enqueued even if we tried to enqueue one job (this shouldn't have any issues regarding uniqueness).

@ivannovosad ivannovosad deleted the fix-unique branch November 25, 2024 14:22
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.

4 participants