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

Add Nova Scheduling Provider #79

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

Conversation

drewatk
Copy link
Contributor

@drewatk drewatk commented May 15, 2023

Adds a new provider, context, and interfaces for a scheduling service to be exposed as a nova context. This will allow react components to schedule units of work to an app's scheduling services.

The interfaces follow TMP's implementation, with generic string ids to track scheduledById and jobId. The interface allows cancelling the job via the job handle returned from Priorities follow those defined by the Prioritized Task Scheduling API within chrome: https://wicg.github.io/scheduling-apis/#sec-task-priorities.

*/
id: string;
/**
* The identifier of the type of code which scheduled the job. Not that this is
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* The identifier of the type of code which scheduled the job. Not that this is
* The identifier of the type of code which scheduled the job. Note that this is

priority: JobPriority;
/**
* The identifier of the type of job. Note that this is not globally
* unique amongst all instance of a job type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* unique amongst all instance of a job type.
* unique amongst all instances of a job type.

NovaSchedulingProviderProps
> = ({ children, scheduling }) => {
return (
<NovaSchedulingContext.Provider value={scheduling}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to add a default value for scheduling to support hosts which simple needs? We don't have any examples in this PR of how we expect a host to hook this, or a default implementation... :)

@@ -0,0 +1,25 @@
export interface NovaScheduling {
Copy link
Contributor

Choose a reason for hiding this comment

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

Take a look at the comments on the other interface files - let's add some inline documentation around what this is and how to use it. At the moment, this is not clear on how a host would fulfill its duty to a component that is using this API.

Without a default implementation, it will also be difficult for component builders to use the API in their development environments - we will need to think through what the simplest path is for someone who wants to support this in Storybook for example...
/**

  • Describes the eventing contract that is used to communicate between UI components and their logical parent within a host application.
  • Usage: components should create small independent packages that define the events they generate using this contract.
  • Logical parents can take a dependency on the event package, and use it to translate the events into appropriate actions within the larger application.
    */


type IJob<TResult> = (() => Promise<TResult>) | (() => TResult);

export interface IJobMetaData {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we are currently hiding details of the scheduler API:

What are the reasons for not exposing these, and would we consider exposing them in future?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The interface should be kept generic to provide scheduling implementation flexibility for consumers.

Operations that aren't currently supported by the API are changing priority and exposing a completion promise. My thought was to keep the API simply at the minimal set of features needed, and then we can add more as required.

@sjwilczynski
Copy link
Contributor

Should we add scheduling provider in test utils so that it has fully mocked nova environment?

@sjwilczynski
Copy link
Contributor

Would be also awesome to add some example component relying scheduling provider to packages/examples :)

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.

3 participants