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

Change interface JobScheduler remove signatures to provide control me… #1322

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kana-dowjones
Copy link

Use case:

When messages are deleted from the scheduler we need original message attributes, like user, timestamp etc.
So the resulting message can have an acting user, who cancels the schedule, and an old user who scheduled the message

The only way to access old messages is to change the signature of a JobScheduler, which is called from SchedulerBroker.send(ProducerBrokerExchange producerExchange, final Message messageSend) method

@kenliao94
Copy link
Contributor

kenliao94 commented Oct 18, 2024

Hi @kana-dowjones why do we need those attributes? Could you elaborate on the benefit of this usecase?

And it would be great to capture this in a Jira ticket as well https://issues.apache.org/jira/projects/AMQ

@kana-dowjones
Copy link
Author

When I said We need those attributes I meant this is our company system-specific case. We have a listener to send notifications about unscheduled messages to interested parties and naturally we need to include some attributes from the original message. Currently, when we delete messages from the scheduler all we have is jobId.

Do you want me to create a Jira ticket?

@kenliao94
Copy link
Contributor

@kana-dowjones yes please create a Jira ticket for further triage.

Personally I don't mind the change, but the change itself should benefit the whole community and not specific to a company requirement. So I am curious about what other people think.

@mattrpav
Copy link
Contributor

Since this is for managing scheduled messages via the messaging API, I think the new methods can be additive, vs a refactor of existing methods which is a SPI breaking change. Management via JMX or other entry point would not have an associated message or messageId to use.

@mattrpav mattrpav self-requested a review October 25, 2024 19:29
@tabish121
Copy link
Contributor

The original intention was to define this API as disjoint from the Messages as that is not really necessary at this level, and given the changes are adding Message only to remove class, why would the other APIs not also take a Message as you create a scheduled message from an inbound Message. Overall the change fells like bad API design, but do as you will.

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