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

Refactor Consumer Module: Introduce JobRunner Trait for Flexible Job Handling #51

Merged
merged 8 commits into from
Feb 18, 2024

Conversation

Kab1r
Copy link
Contributor

@Kab1r Kab1r commented Jan 31, 2024

Background

I need to have access to some initialized configuration / state for my Handler.
While I could use once_cell to initialize static variables to hold a configuration, it isn't particularly idiomatic.

Summary of Changes

This PR introduces a change to the Consumer module, converting the JobRunner type into a trait.
The JobRunner trait is now implemented for types that can handle jobs, providing more flexibility for handling jobs within a Consumer.

Example Usage

use faktory::{ConsumerBuilder, JobRunner, Job};
use std::io;

struct MyHandler {
    config: String,
}
impl JobRunner<io::Error> for MyHandler {
   fn run(&self, job: Job) -> Result<(), io::Error> {
      println!("config: {}", self.config);
      println!("job: {:?}", job);
      Ok(())
  }
}

let mut c = ConsumerBuilder::default();
let handler = MyHandler {
   config: "foo".to_string(),
};
c.register_runner("bar", handler);
let mut c = c.connect(None).unwrap();
if let Err(e) = c.run(&["default"]) {
    println!("worker failed: {}", e);
}

This change is Reviewable

src/consumer/mod.rs Outdated Show resolved Hide resolved
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (cdf15d3) 68.4% compared to head (6ed657f) 68.2%.

Additional details and impacted files
Files Coverage Δ
src/consumer/mod.rs 68.4% <57.1%> (-1.3%) ⬇️

@rustworthy rustworthy requested a review from jonhoo February 4, 2024 10:51
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

I totally see the need! Left some comments on how to achieve it though.

src/consumer/mod.rs Outdated Show resolved Hide resolved
src/consumer/mod.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
- removed breaking change: introduced `register_runner`
- replaced `E` generic type parameter with asssociated type in `JobRunner` trait
src/consumer/mod.rs Outdated Show resolved Hide resolved
@Kab1r Kab1r requested a review from jonhoo February 6, 2024 08:33
src/consumer/mod.rs Outdated Show resolved Hide resolved
src/consumer/mod.rs Outdated Show resolved Hide resolved
src/consumer/mod.rs Outdated Show resolved Hide resolved
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

Almost there — just a couple of nits left!

src/consumer/mod.rs Show resolved Hide resolved
@Kab1r Kab1r requested a review from jonhoo February 12, 2024 07:43
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

Just one more bit to avoid the box, and then I think we're done!

src/consumer/mod.rs Outdated Show resolved Hide resolved
src/consumer/mod.rs Show resolved Hide resolved
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

Looks good to me — thanks for persisting!

@jonhoo jonhoo merged commit 248c34e into jonhoo:main Feb 18, 2024
17 of 18 checks passed
@jonhoo
Copy link
Owner

jonhoo commented Feb 18, 2024

Released in 0.12.5 🎉

@rustworthy rustworthy mentioned this pull request Apr 27, 2024
7 tasks
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