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

feat: Request, Job Hooks {Before/After} #19971

Merged
merged 12 commits into from
Feb 17, 2023

Conversation

gavindsouza
Copy link
Collaborator

@gavindsouza gavindsouza commented Feb 10, 2023

This PR introduces [before, after] hooks for Requests & Background Jobs.

The hooks will be executed while the context of locals (request, response, flags, etc), the DB connection & such are maintained.

Request Hooks

These hooks allow you to run code before and after a request is made. These hooks can be defined as list[str] in which the function's dotted paths are defined.

The before hook is executed at the end of request initialization. This means accessing frappe.request in your before_request functions will be enough to get all relevant context.

The after hook is executed after the response object is built and database syncing & cleanup tasks are completed, but before the DB connection is closed. In case of a request unsupported by the WSGI app, the response object may not exist. However, for most purposes accessing frappe.response should be enough. The request & response objects are also passed into the after_request hooks if your function needs it.

Here are the request hooks that Frappe itself implements that you can find in it's codebase:

before_request = [
	"frappe.recorder.record",
	"frappe.monitor.start",
	"frappe.rate_limiter.apply",
]
after_request = ["frappe.rate_limiter.update", "frappe.monitor.stop", "frappe.recorder.dump"]

Background Job Hooks

These hooks allow you to run code before and after a background job is executed. These hooks can be defined as list[str] in which the function's dotted paths are defined.

The before and after hooks receive the job's dotted name (method), kwargs (kwargs) and an extra kwarg (transaction_type for before_job, result for after_job), if your function will accept them.

Here are the job hooks that Frappe itself implements that you can find in it's codebase:

before_job = [
	"frappe.monitor.start",
]
after_job = [
	"frappe.monitor.stop",
	"frappe.utils.file_lock.release_document_locks",
]

Docs: https://frappeframework.com/docs/v13/user/en/python-api/hooks/edit-wiki?wiki_page_patch=bcf13033cc

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Merging #19971 (985e091) into develop (2d41609) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #19971      +/-   ##
===========================================
+ Coverage    63.67%   63.69%   +0.01%     
===========================================
  Files          753      753              
  Lines        68316    68318       +2     
  Branches      6151     6151              
===========================================
+ Hits         43500    43512      +12     
+ Misses       21305    21295      -10     
  Partials      3511     3511              
Flag Coverage Δ
server 68.86% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@gavindsouza gavindsouza marked this pull request as ready for review February 10, 2023 08:41
@gavindsouza gavindsouza requested review from a team and phot0n and removed request for a team February 10, 2023 08:41
frappe/app.py Outdated Show resolved Hide resolved
frappe/app.py Outdated Show resolved Hide resolved
@netchampfaris
Copy link
Contributor

@gavindsouza

image

Copy link
Contributor

@netchampfaris netchampfaris left a comment

Choose a reason for hiding this comment

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

The framework should also use the same API for monitor/record operations?

hooks.py

before_request = [
  'frappe.rate_limiter.apply',
  'frappe.monitor.start',
  'frappe.recorder.record'
]
after_request = [
  'frappe.rate_limiter.update',
  'frappe.monitor.stop',
  'frappe.recorder.dump'
]

Also, rename after_request fn to sync_database to better match functionality
Moved before/after tasks in Requests as hooks for:
- monitor
- rate_limiter
- recorder

Moved before/after tasks in Jobs as hooks for:
- monitor
- releasing document locks
@ankush
Copy link
Member

ankush commented Feb 16, 2023

Test failures seem related @gavindsouza reproducible locally too.

* it can fetch most relevant details via response object
* Exceptions not supported by Frappe's WSGI (unsupported HTTP methods) may not be accessible to the after_request hooks - but the lack of active response may be an indicator / and peeking in the request
ankush pushed a commit that referenced this pull request Feb 20, 2023
* feat: Before/After Request Hooks

(cherry picked from commit d6a41cd)

* feat: Befor/After Job Hooks

(cherry picked from commit 34731d1)

* test: Add tests for job sanity + hooks

(cherry picked from commit 0d547cf)

* test: Added test for before/after request hook

(cherry picked from commit 55b31fc)

* fix(background_jobs): Pass retval in execute_job

(cherry picked from commit 83f3cf1)

* fix: Add request, job events in hooks boilerplate

(cherry picked from commit ea62156)

* fix: Pass result of job to after_job hooks

(cherry picked from commit 2ada98f)

* fix(app): Move after_request hook inside finally block

Also, rename after_request fn to sync_database to better match functionality

(cherry picked from commit 6d70b5e)

* refactor: Move before/after tasks as hooks

Moved before/after tasks in Requests as hooks for:
- monitor
- rate_limiter
- recorder

Moved before/after tasks in Jobs as hooks for:
- monitor
- releasing document locks

(cherry picked from commit fe26c54)

* fix(after_hook): Don't pass exception object to hook

* it can fetch most relevant details via response object
* Exceptions not supported by Frappe's WSGI (unsupported HTTP methods) may not be accessible to the after_request hooks - but the lack of active response may be an indicator / and peeking in the request

(cherry picked from commit 5d0bd51)

* chore: remove unsupported args from test

---------

Co-authored-by: Gavin D'souza <gavin18d@gmail.com>
frappe-pr-bot pushed a commit that referenced this pull request Feb 21, 2023
# [14.26.0](v14.25.4...v14.26.0) (2023-02-21)

### Bug Fixes

* checkbox is getting squeezed if label is long ([7992f28](7992f28))
* do not allow restricted fieldnames for custom fields (backport [#19928](#19928)) ([#20103](#20103)) ([3e28172](3e28172))
* do not show help-icon in report view (datatable) ([c4bfa7e](c4bfa7e))
* don't print multiple print statements when database settings dont match with expected settings ([#18492](#18492)) ([c6557e0](c6557e0))
* drop mariadb 10.2 checks ([8ca72b1](8ca72b1))
* help-icon taking space if doc url is not set ([2ceb26f](2ceb26f))
* Hide perm level fields for Section, Column and Tab Breaks ([#20084](#20084)) ([#20089](#20089)) ([834a553](834a553))
* hide published checkbox ([9e92cd7](9e92cd7))
* Interface DatabaseQuery to virtual doctypes' get_list (backport [#19854](#19854)) ([#19925](#19925)) ([5c43835](5c43835))
* link validation fetch from virtual doc ([#20055](#20055)) ([ba641c2](ba641c2))
* missing report permission stops boot (backport [#20095](#20095)) ([#20099](#20099)) ([2bc63c7](2bc63c7))
* Notification JS relied on leaked locals ([#20048](#20048)) ([#20049](#20049)) ([3339cd8](3339cd8))
* restrict DocType layout permissions ([#20028](#20028)) ([#20030](#20030)) ([7887388](7887388))
* thread-unsafe shared class attribute on QB engine ([#20052](#20052)) ([b1c94b3](b1c94b3))
* traceback sanitizer got extra positional args ([424baed](424baed))
* unable to upload image in responsive mode ([#19963](#19963)) ([c096094](c096094))
* **UX:** show message when form is read only ([#20077](#20077)) ([#20082](#20082)) ([cd8c06c](cd8c06c))
* Warn about incompatible mariadb versions ([917a3ec](917a3ec))

### Features

* Allow app_include_js and app_include_css via site config (backport [#18841](#18841)) ([#20067](#20067)) ([4b77b96](4b77b96))
* **Calendar:** Add a new option `convertToUserTz` to address timezone inconsistencies ([2b42510](2b42510))
* publish button in blog post form ([1a31f94](1a31f94))
* Request, Job Hooks {Before/After} (backport [#19971](#19971)) ([#20074](#20074)) ([6a7825b](6a7825b))
ankush added a commit that referenced this pull request Feb 22, 2023
* feat: Before/After Request Hooks

(cherry picked from commit d6a41cd)

# Conflicts:
#	frappe/app.py

* feat: Befor/After Job Hooks

(cherry picked from commit 34731d1)

# Conflicts:
#	frappe/utils/background_jobs.py

* test: Add tests for job sanity + hooks

(cherry picked from commit 0d547cf)

# Conflicts:
#	frappe/tests/test_background_jobs.py

* test: Added test for before/after request hook

(cherry picked from commit 55b31fc)

# Conflicts:
#	frappe/tests/test_api.py

* fix(background_jobs): Pass retval in execute_job

(cherry picked from commit 83f3cf1)

* fix: Add request, job events in hooks boilerplate

(cherry picked from commit ea62156)

# Conflicts:
#	frappe/utils/boilerplate.py

* fix: Pass result of job to after_job hooks

(cherry picked from commit 2ada98f)

* fix(app): Move after_request hook inside finally block

Also, rename after_request fn to sync_database to better match functionality

(cherry picked from commit 6d70b5e)

# Conflicts:
#	frappe/app.py

* refactor: Move before/after tasks as hooks

Moved before/after tasks in Requests as hooks for:
- monitor
- rate_limiter
- recorder

Moved before/after tasks in Jobs as hooks for:
- monitor
- releasing document locks

(cherry picked from commit fe26c54)

# Conflicts:
#	frappe/hooks.py
#	frappe/utils/background_jobs.py

* fix(after_hook): Don't pass exception object to hook

* it can fetch most relevant details via response object
* Exceptions not supported by Frappe's WSGI (unsupported HTTP methods) may not be accessible to the after_request hooks - but the lack of active response may be an indicator / and peeking in the request

(cherry picked from commit 5d0bd51)

* fix: Resolve conflicts

* test: Allow params as None in FrappeAPITestCase

* fix: Set matching mariaDB user passwords

There seemed to be a mismatch of id passwords between what's written in
the site_config and mariadb install.sh blocks

---------

Co-authored-by: Gavin D'souza <gavin18d@gmail.com>
Co-authored-by: Ankush Menat <ankush@frappe.io>
@ascorbic-acid
Copy link

Thanks alot this is very useful this

developmentforpeople pushed a commit to Ayuda-Efectiva/frappe that referenced this pull request Feb 24, 2023
…pe#20074)

* feat: Before/After Request Hooks

(cherry picked from commit d6a41cd)

* feat: Befor/After Job Hooks

(cherry picked from commit 34731d1)

* test: Add tests for job sanity + hooks

(cherry picked from commit 0d547cf)

* test: Added test for before/after request hook

(cherry picked from commit 55b31fc)

* fix(background_jobs): Pass retval in execute_job

(cherry picked from commit 83f3cf1)

* fix: Add request, job events in hooks boilerplate

(cherry picked from commit ea62156)

* fix: Pass result of job to after_job hooks

(cherry picked from commit 2ada98f)

* fix(app): Move after_request hook inside finally block

Also, rename after_request fn to sync_database to better match functionality

(cherry picked from commit 6d70b5e)

* refactor: Move before/after tasks as hooks

Moved before/after tasks in Requests as hooks for:
- monitor
- rate_limiter
- recorder

Moved before/after tasks in Jobs as hooks for:
- monitor
- releasing document locks

(cherry picked from commit fe26c54)

* fix(after_hook): Don't pass exception object to hook

* it can fetch most relevant details via response object
* Exceptions not supported by Frappe's WSGI (unsupported HTTP methods) may not be accessible to the after_request hooks - but the lack of active response may be an indicator / and peeking in the request

(cherry picked from commit 5d0bd51)

* chore: remove unsupported args from test

---------

Co-authored-by: Gavin D'souza <gavin18d@gmail.com>
developmentforpeople pushed a commit to Ayuda-Efectiva/frappe that referenced this pull request Feb 24, 2023
# [14.26.0](frappe/frappe@v14.25.4...v14.26.0) (2023-02-21)

### Bug Fixes

* checkbox is getting squeezed if label is long ([7992f28](frappe@7992f28))
* do not allow restricted fieldnames for custom fields (backport [frappe#19928](frappe#19928)) ([frappe#20103](frappe#20103)) ([3e28172](frappe@3e28172))
* do not show help-icon in report view (datatable) ([c4bfa7e](frappe@c4bfa7e))
* don't print multiple print statements when database settings dont match with expected settings ([frappe#18492](frappe#18492)) ([c6557e0](frappe@c6557e0))
* drop mariadb 10.2 checks ([8ca72b1](frappe@8ca72b1))
* help-icon taking space if doc url is not set ([2ceb26f](frappe@2ceb26f))
* Hide perm level fields for Section, Column and Tab Breaks ([frappe#20084](frappe#20084)) ([frappe#20089](frappe#20089)) ([834a553](frappe@834a553))
* hide published checkbox ([9e92cd7](frappe@9e92cd7))
* Interface DatabaseQuery to virtual doctypes' get_list (backport [frappe#19854](frappe#19854)) ([frappe#19925](frappe#19925)) ([5c43835](frappe@5c43835))
* link validation fetch from virtual doc ([frappe#20055](frappe#20055)) ([ba641c2](frappe@ba641c2))
* missing report permission stops boot (backport [frappe#20095](frappe#20095)) ([frappe#20099](frappe#20099)) ([2bc63c7](frappe@2bc63c7))
* Notification JS relied on leaked locals ([frappe#20048](frappe#20048)) ([frappe#20049](frappe#20049)) ([3339cd8](frappe@3339cd8))
* restrict DocType layout permissions ([frappe#20028](frappe#20028)) ([frappe#20030](frappe#20030)) ([7887388](frappe@7887388))
* thread-unsafe shared class attribute on QB engine ([frappe#20052](frappe#20052)) ([b1c94b3](frappe@b1c94b3))
* traceback sanitizer got extra positional args ([424baed](frappe@424baed))
* unable to upload image in responsive mode ([frappe#19963](frappe#19963)) ([c096094](frappe@c096094))
* **UX:** show message when form is read only ([frappe#20077](frappe#20077)) ([frappe#20082](frappe#20082)) ([cd8c06c](frappe@cd8c06c))
* Warn about incompatible mariadb versions ([917a3ec](frappe@917a3ec))

### Features

* Allow app_include_js and app_include_css via site config (backport [frappe#18841](frappe#18841)) ([frappe#20067](frappe#20067)) ([4b77b96](frappe@4b77b96))
* **Calendar:** Add a new option `convertToUserTz` to address timezone inconsistencies ([2b42510](frappe@2b42510))
* publish button in blog post form ([1a31f94](frappe@1a31f94))
* Request, Job Hooks {Before/After} (backport [frappe#19971](frappe#19971)) ([frappe#20074](frappe#20074)) ([6a7825b](frappe@6a7825b))
developmentforpeople pushed a commit to developmentforpeople/frappe that referenced this pull request Feb 25, 2023
…pe#20074)

* feat: Before/After Request Hooks

(cherry picked from commit d6a41cd)

* feat: Befor/After Job Hooks

(cherry picked from commit 34731d1)

* test: Add tests for job sanity + hooks

(cherry picked from commit 0d547cf)

* test: Added test for before/after request hook

(cherry picked from commit 55b31fc)

* fix(background_jobs): Pass retval in execute_job

(cherry picked from commit 83f3cf1)

* fix: Add request, job events in hooks boilerplate

(cherry picked from commit ea62156)

* fix: Pass result of job to after_job hooks

(cherry picked from commit 2ada98f)

* fix(app): Move after_request hook inside finally block

Also, rename after_request fn to sync_database to better match functionality

(cherry picked from commit 6d70b5e)

* refactor: Move before/after tasks as hooks

Moved before/after tasks in Requests as hooks for:
- monitor
- rate_limiter
- recorder

Moved before/after tasks in Jobs as hooks for:
- monitor
- releasing document locks

(cherry picked from commit fe26c54)

* fix(after_hook): Don't pass exception object to hook

* it can fetch most relevant details via response object
* Exceptions not supported by Frappe's WSGI (unsupported HTTP methods) may not be accessible to the after_request hooks - but the lack of active response may be an indicator / and peeking in the request

(cherry picked from commit 5d0bd51)

* chore: remove unsupported args from test

---------

Co-authored-by: Gavin D'souza <gavin18d@gmail.com>
developmentforpeople pushed a commit to developmentforpeople/frappe that referenced this pull request Feb 25, 2023
# [14.26.0](frappe/frappe@v14.25.4...v14.26.0) (2023-02-21)

### Bug Fixes

* checkbox is getting squeezed if label is long ([7992f28](frappe@7992f28))
* do not allow restricted fieldnames for custom fields (backport [frappe#19928](frappe#19928)) ([frappe#20103](frappe#20103)) ([3e28172](frappe@3e28172))
* do not show help-icon in report view (datatable) ([c4bfa7e](frappe@c4bfa7e))
* don't print multiple print statements when database settings dont match with expected settings ([frappe#18492](frappe#18492)) ([c6557e0](frappe@c6557e0))
* drop mariadb 10.2 checks ([8ca72b1](frappe@8ca72b1))
* help-icon taking space if doc url is not set ([2ceb26f](frappe@2ceb26f))
* Hide perm level fields for Section, Column and Tab Breaks ([frappe#20084](frappe#20084)) ([frappe#20089](frappe#20089)) ([834a553](frappe@834a553))
* hide published checkbox ([9e92cd7](frappe@9e92cd7))
* Interface DatabaseQuery to virtual doctypes' get_list (backport [frappe#19854](frappe#19854)) ([frappe#19925](frappe#19925)) ([5c43835](frappe@5c43835))
* link validation fetch from virtual doc ([frappe#20055](frappe#20055)) ([ba641c2](frappe@ba641c2))
* missing report permission stops boot (backport [frappe#20095](frappe#20095)) ([frappe#20099](frappe#20099)) ([2bc63c7](frappe@2bc63c7))
* Notification JS relied on leaked locals ([frappe#20048](frappe#20048)) ([frappe#20049](frappe#20049)) ([3339cd8](frappe@3339cd8))
* restrict DocType layout permissions ([frappe#20028](frappe#20028)) ([frappe#20030](frappe#20030)) ([7887388](frappe@7887388))
* thread-unsafe shared class attribute on QB engine ([frappe#20052](frappe#20052)) ([b1c94b3](frappe@b1c94b3))
* traceback sanitizer got extra positional args ([424baed](frappe@424baed))
* unable to upload image in responsive mode ([frappe#19963](frappe#19963)) ([c096094](frappe@c096094))
* **UX:** show message when form is read only ([frappe#20077](frappe#20077)) ([frappe#20082](frappe#20082)) ([cd8c06c](frappe@cd8c06c))
* Warn about incompatible mariadb versions ([917a3ec](frappe@917a3ec))

### Features

* Allow app_include_js and app_include_css via site config (backport [frappe#18841](frappe#18841)) ([frappe#20067](frappe#20067)) ([4b77b96](frappe@4b77b96))
* **Calendar:** Add a new option `convertToUserTz` to address timezone inconsistencies ([2b42510](frappe@2b42510))
* publish button in blog post form ([1a31f94](frappe@1a31f94))
* Request, Job Hooks {Before/After} (backport [frappe#19971](frappe#19971)) ([frappe#20074](frappe#20074)) ([6a7825b](frappe@6a7825b))
frappe-pr-bot pushed a commit that referenced this pull request Feb 28, 2023
# [13.50.0](v13.49.3...v13.50.0) (2023-02-28)

### Bug Fixes

* german translations (backport [#17663](#17663)) ([#20114](#20114)) ([1a38314](1a38314))
* hide invalid filters for 'HTML Editor' and 'Markdown Editor' ([#20119](#20119)) ([#20127](#20127)) ([1c69d43](1c69d43))
* prevent overwriting of the warnings by PyPDF2 ([#20170](#20170)) ([dca7c38](dca7c38))
* raise_exception=False for has_permission in filter_allowed_users ([#20113](#20113)) ([#20122](#20122)) ([23137f2](23137f2))
* Reload frm doc doc_update trigger to avoid timestamp conflict ([df4d3a2](df4d3a2))
* support for different delimiter for timeline email linking (backport [#19751](#19751)) ([#20154](#20154)) ([ea78695](ea78695))
* **UX:** Freeze form while applying workflow ([db93700](db93700))
* **UX:** invalid filters, incorrect std fieldtypes (backport [#19657](#19657)) ([#20118](#20118)) ([a612baa](a612baa))
* **workflow:** move dom unfreeze to `finally` (backport [#20161](#20161)) ([#20176](#20176)) ([14abb0e](14abb0e))

### Features

* Request, Job Hooks {Before/After} (backport [#19971](#19971)) ([#20073](#20073)) ([17adda2](17adda2))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants