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

Provide provider_job_id to qu adapter. #20064

Merged
merged 1 commit into from May 8, 2015
Merged

Provide provider_job_id to qu adapter. #20064

merged 1 commit into from May 8, 2015

Conversation

kddnewton
Copy link
Contributor

Further work to provide provider_job_id for queue adapters.

@rafaelfranca
Copy link
Member

Don't forget the CHANGELOG

@kddnewton
Copy link
Contributor Author

@rafaelfranca - thanks! Just updated it.

Further work to provide provider_job_id for queue adapters.
@rafaelfranca
Copy link
Member

Tests seems to be broken.

@kddnewton
Copy link
Contributor Author

It's not displaying the travis errors for me, do you have a link? It was broken until about 20 minutes ago - is it still now? I had to add the "unless qu_job.nil?" because of the different backends.

@kddnewton
Copy link
Contributor Author

@rafaelfranca any idea how to proceed? Both the regular and integration tests are passing for me, so I don't exactly know what to do. Thanks.

@matthewd matthewd closed this May 8, 2015
@matthewd matthewd reopened this May 8, 2015
@kddnewton
Copy link
Contributor Author

Thanks @matthewd - looks like it's good now.


test 'should supply a provider_job_id when available for delayed jobs' do
skip unless adapter_is?(:delayed_job, :sidekiq, :que)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you forgot to add the provider_job_id for qu in the enqueed_at method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah qu doesn't support scheduled jobs, so I didn't include that in the test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sorry, I only saw the first line of def enqueue_at(job, timestamp, *args) #:nodoc: did not read the full method body.

rafaelfranca added a commit that referenced this pull request May 8, 2015
Provide provider_job_id to qu adapter.
@rafaelfranca rafaelfranca merged commit fca8a7a into rails:master May 8, 2015
@kddnewton kddnewton deleted the qu_provider_job_id branch May 8, 2015 18:15
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.

None yet

4 participants