Skip to content

Conversation

@amimart
Copy link
Contributor

@amimart amimart commented Oct 7, 2025

Purpose of Changes and their Description

Add the emissions of the events below in the x/scheduler module:

  • TaskScheduledEvent: Emitted when a task is scheduled or rescheduled at a specific time (i.e. a task can be created without being scheduled, in that case the event is not emitted);
  • TaskUnscheduledEvent: Emitted when a task that was scheduled, is cancelled or unscheduled;
  • TaskExecutedEvent: Emitted when a task is executed.;

Are these changes tested and documented?

  • If tested, please describe how. If not, why tests are not needed.
  • If documented, please describe where. If not, describe why docs are not needed.
  • Added to Unreleased section of CHANGELOG.md?

@amimart amimart changed the base branch from dev to arnaud/design-scheduler October 7, 2025 09:39
@github-actions
Copy link

github-actions bot commented Oct 7, 2025

The latest Buf updates on your PR. Results from workflow Buf Linter / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed⏩ skippedOct 21, 2025, 3:41 PM

@amimart amimart force-pushed the arnaud/design-scheduler branch from a96e5b7 to ee375bb Compare October 20, 2025 17:47
@amimart amimart force-pushed the arnaud/engn-4255-expose-events-on-xscheduler-module branch from 8d3943c to 5093368 Compare October 20, 2025 19:57
@amimart amimart force-pushed the arnaud/design-scheduler branch from 9b9c432 to ada7012 Compare October 20, 2025 20:27
@amimart amimart force-pushed the arnaud/engn-4255-expose-events-on-xscheduler-module branch from 5093368 to c5936b1 Compare October 20, 2025 20:32
xmariachi
xmariachi previously approved these changes Oct 20, 2025
Base automatically changed from arnaud/design-scheduler to dev October 21, 2025 11:03
@amimart amimart dismissed xmariachi’s stale review October 21, 2025 11:03

The base branch was changed.

@amimart amimart force-pushed the arnaud/engn-4255-expose-events-on-xscheduler-module branch from c5936b1 to c487365 Compare October 21, 2025 12:32
@amimart amimart marked this pull request as ready for review October 21, 2025 12:36
xmariachi
xmariachi previously approved these changes Oct 21, 2025
Copy link
Contributor

@xmariachi xmariachi left a comment

Choose a reason for hiding this comment

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

lgtm!

return err
}

oldScheduleKey := getTaskScheduleKey(task)
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually we return the struct + err on the keeper's get methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh but the task schedule key can be nil it's not an error, but it's an impossible/forbidden case when we execute a task

Comment on lines +484 to +486
evt, err := cosmostypes.ParseTypedEvent(e)
require.NoError(t, err)
parsed = append(parsed, evt)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe consider extracting this for parsing single events, in case we might use it. maybe in a util? just a thought

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.

5 participants