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

please export delete function #20

Closed
chengjianxi opened this issue Mar 21, 2022 · 6 comments
Closed

please export delete function #20

chengjianxi opened this issue Mar 21, 2022 · 6 comments

Comments

@chengjianxi
Copy link

func (conn *Conn) delete(ctx context.Context, job *Job) error {
	ctx, span := trace.StartSpan(ctx, "github.com/prep/beanstalk/Conn.delete")
	defer span.End()

	_, _, err := conn.lcommand(ctx, "delete %d", job.ID)
	return err
}
@prep
Copy link
Owner

prep commented Mar 21, 2022

Hey JX, can you tell me the use-case in which you'd need it? Pretty much the entire API of this package returns a *beanstalk.Job{} on which you can call job.Delete(). Doesn't that suit your purpose?

@chengjianxi
Copy link
Author

Yes, I insert a delayed message and delete it before it is consumed.
The usage scenario is: send a message to the client, then insert the delayed message to beanstalk. If the client ACK within a certain period of time (eg. 30 seconds), the message in beanstalk will be deleted, otherwise the message will be resent. Because messages are asynchronously, I think *beanstalk.Job{} is not fit.

@prep
Copy link
Owner

prep commented Mar 26, 2022

Hey JX, thanks for laying out your use-case 👍 I've exported the Delete() method on Conn{} and created a new release.

@chengjianxi
Copy link
Author

Thank you !

@chengjianxi
Copy link
Author

chengjianxi commented Mar 29, 2022

I think Producer's connection pool is great, so I put the Delete method in Producer.

#22

@prep
Copy link
Owner

prep commented Apr 8, 2022

Sorry for the late response, JX.

I don't think adding a Delete() method on Producer{} is such a good idea. Category-wise, Consumer{} would have been better, but it's really besides the point because both are "pools" of connections and you cannot just pick a random connection (as the code does) and perform a Delete() operation on it.

The reason for this is that your pool can refer to different beanstalk servers and each of them have their own count of job IDs. So deleting job 123 on a random server has a chance of either 1) not finding the job you were referring to, or worse 2) delete a different job than you intended.

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

No branches or pull requests

2 participants