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 new "steps" background task type #484

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

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Aug 16, 2021

Summary

This PR adds a new background task type, available through the submit_steps convenience function, returning a StepsFuture. Like the existing submit_progress, it can be used to run a cancellable progress-reporting background task. Unlike submit_progress, it offers a richer interface for progress reporting to the background task.

The plan is to add documentation, executable examples, and in particular example progress dialogs in a separate PR.

Usage example

The task type allows submission of a function like the following:

def upload_files(filenames, reporter):
    for filename in filenames:
        reporter.step(f"Uploading file {filename}")
        do_upload(filename)
    reporter.stop(f"All files uploaded")

This task would be submitted using a call resembling:

steps_future = submit_steps(traits_executor, len(filenames), "Uploading files", upload_files, filenames)

The returned value steps_future is an instance of StepsFuture, and has message, total and complete traits that are updated with information about the currently-executing step, the total number of files to upload, and the number of uploads completed, respectively. These values could then be observed by a suitable progress dialog (an example such dialog can be found in #367).

Details

The PR adds three items to traits_futures.api:

  • a new top-level function submit_steps, allowing submission of the progress-reporting task
  • a new future type, StepsFuture, for the return value of submit_steps. This inherits from BaseFuture and has the same future state as other task types, but also has public traits message, total and complete representing the progress state
  • an interface IStepsReporter that describes the extra reporter argument that's supplied to the background task. This is potentially useful for projects that want to supply their own fake IStepsReporter instance while testing the functionality of the background task outside the Traits Futures framework

Implementation notes

The state of the progress at any given time is represented by a StepsState instance: this is a namedtuple subclass that holds three integers and a string:

  • the total units of work that the task will process (total)
  • the units of work processed so far (complete)
  • the units of work represented by the currently in-progress step (pending)
  • a message for the current step (message)

Both the StepsFuture and the StepsReporter hold copies of the current state; every time the reporter's step or stop method is called, the local copy of the state is updated and then a message sent to the future with the updated state. When the future receives that message, it updates its own copy of the state. The three public traits (total, complete and message) on the StepsFuture` are properties based on the private internal state. This avoids potential issues with listeners to those traits seeing different values depending on the order of updates.

Further context

This PR is extracted from #367. See #367 for executable examples that make use of submit_steps and the StepsFuture.

Additional features planned

Some additional things that I think would be useful, but aren't planned to be included in this PR:

  • docs and examples, including a full-fledged progress dialog suitable that acts as a view for the StepsFuture model
  • a StepsReporter.check method that just checks for cancellation, and doesn't report any progress updates
  • a NullStepsReporter that implements the IStepsReporter interface, but does nothing, for use in testing
  • it may make sense to add a second communications channel that can be used to report partial results; we have existing uses of submit_progress where the progress payload is used to transmit partial results

@rahulporuri rahulporuri self-requested a review August 16, 2021 17:02
Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

Mostly LGTM with one minor question.

#: Named arguments to be passed to the callable, excluding the "reporter"
#: named argument. The "reporter" argument will be supplied through the
#: execution machinery.
kwargs = Dict(Str())
Copy link
Contributor

Choose a reason for hiding this comment

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

Q : does this mean that the keys of the dictionary are strings and implicitly the value trait type isn't defined?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Possibly I should make that explicit by adding Any() as a the value type.

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