-
Notifications
You must be signed in to change notification settings - Fork 37
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
Implement execution hooks framework #189
Conversation
This is a prototype implementation for the API proposed in issue #28.
In the snapshot hook module.
To avoid the need to implement a class and also avoid forcing of a specific function name.
This makes it easier to integrate those hook systems with git tracking.
Instead of storing metadata in the commit message.
@bdice You were not assigned as reviewer, but might still be interested in this feature. |
Note: This feature won't work with "exec operations", but since these are going away anyways, I think it's safe to introduce this feature regardless. |
@cbkerr Is this ready for another review? |
I read through some of this PR with @klywang. We're scheduling a time to dig deeper on this and consider the architecture of the code. I have two broad considerations:
|
@bdice @cbkerr @klywang most of my comments are still relevant after #482 and addressing them should make the merge easier later. I'd prioritize docs/addressing open discussions over further refactoring work on this branch while other work happens on the main branch. I definitely agree that we should aim to simplify before finalizing this feature, but I think writing docs should come first so that @cbkerr and @klywang can clearly articulate the goals of this PR first since understanding how it should work is critical to writing the best version of the code. |
Codecov Report
@@ Coverage Diff @@
## master #189 +/- ##
==========================================
- Coverage 73.40% 69.61% -3.80%
==========================================
Files 30 37 +7
Lines 2967 3268 +301
Branches 563 591 +28
==========================================
+ Hits 2178 2275 +97
- Misses 644 845 +201
- Partials 145 148 +3
Continue to review full report at Codecov.
|
Now that #508 has been merged, we need to decide whether to implement the suggested hooks from this PR. I think the best way to do that would be to close this PR and open a new issue listing the proposed hooks and linking to this PR for reference implementations that could be adapted in separate PRs.
|
@bdice This can be closed, right? |
The only reason we left it open was because this PR also implements the specific hook classes that were not included in the general framework that was merged. |
Closed see #637 for further discussion. |
Description
This pull request implements an execution hooks framework that enables users to execute certain functions on start, on finish, on success, and on fail of the execution of an operation.
Resolves #28, resolves #14.
Motivation and Context
The motivation for this framework is to enable users to automatically execute certain functions with operations for example for logging and tracking purposes. For example, to log when a specific operation is executed, a user can provide the following function:
The framework comes with a selection of pre-implemented hook systems to cover all the currently expected use cases, roughly in order of expensiveness:
LogOperation
- Log basic information about the execution of operations to a log file.TrackOperations
- Keep detailed metadata records about the state of the project root directory and the operation, including directives, to a log file, optionally in conjunction with git.SnapshotProject
- Create a snapshot of the project root directory to keep track of the code used for the execution of an operation.TrackWorkspaceWithGit
- The workspace is treated as a git repository and automatically committed to before and after the execution of an operation. The can be done either on a per-job basis or workspace global.A hook system is meant to describe a collection of hook functions that together achieve a specific purpose. The shipped hook systems are implemented as classes that can be installed project-wide via the respective
install_hooks
function.High-level API
Hooks can be installed either on a per-operation or a per-project basis. In this way users have the option to execute hook functions either with specific operations or with all operations of a project.
Furthermore, there are two ways that hooks can be installed. One way is directly in Python, for example within the
__main__
clause of theproject.py
file:Alternatively, hooks can also be installed through the (project) configuration file:
It is assumed that the entities provided here, are callable, i.e., are either a function or a functor. The first argument must be the instance of
FlowProject
.Low-level API
The
FlowProject
class has ahooks
attribute with four lists:on_start
,on_finish
,on_success
, andon_fail
, which can be appended to. Hook functions installed in this way are executed for all operations.Furthermore, hook functions can be installed for individual operations either with a decorator:
or by passing a dictionary to the
add_operation()
function:project.add_operation(..., hooks=dict(on_start=my_hook_function))
.Custom hook systems
Users can very easily implement their own hook systems and install them in similar manner.
For example:
This could then be installed either in the project module:
or via the configuration file:
Types of Changes
1The change breaks (or has the potential to break) existing functionality.
Checklist:
If necessary: