Skip to content

[DO NOT MERGE] Renamed Scheduler APIs to better reflect their usage. #116

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

shmuelk
Copy link
Collaborator

@shmuelk shmuelk commented May 4, 2025

The PR renames the two APIs of the Scheduler. In particular:

  • Scheduler.Schedule has been renamed scheduler.OnRequest to better indicate that it is invoked during request processing.
  • Scheduler.RunPostResponsePlugins has been renamed to Scheduler.OnResponse to both indicate that it is invoked during response processing and to better decouple it from PostResponse plugin handling

In addition the code in the function Scheduler.OnResponse that is specific to PostResponse plugin handling has been moved to a new private function Scheduler.runPostResponsePlugins, in the style of th functions that drive the handling of the other plugins.

shmuelk added 2 commits May 4, 2025 17:25
…o better indicate that it is invoked for response processing. Refactored function to separate out explicit PostResponse handling.
@shmuelk shmuelk requested a review from elevran May 4, 2025 14:33
Copy link
Collaborator

@elevran elevran left a comment

Choose a reason for hiding this comment

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

I would prefer to not change Schedule, but rather add another interface type (ie and not a function on an existing interface) for doing the response processing.

@elevran
Copy link
Collaborator

elevran commented May 6, 2025

/hold
Should minimize changes to scheduler and upstream

@elevran elevran changed the title Renamed Scheduler APIs to better reflect their usage. [DO NOT MERGE] Renamed Scheduler APIs to better reflect their usage. May 6, 2025
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