Yes, I think so. Changes to hooks are picked up on restart as far as I can tell, so migrate probably isn’t as good as restart (in production) or a kill([CTRL-C]) and start in develop mode.
Yes @tmatteson and @BhupeshGupta, you are right. You dont need bench migrate. But bench restart or stopping and starting the process in developer mode will do the job.
The problem was something else.
I have 2 bench versions on my machine. One is frappe 10.1.35 and other is 10.1.29. I was trying on frappe 10.1.35 where this is not working. After switching to 10.1.29, the above worked. I digged at bit into frappe code and found this difference in jinja.py in both the versions of frappe:
In version 10.1.29 (where the filter works) :
...
def set_filters(jenv):
import frappe
from frappe.utils import global_date_format, cint, cstr, flt, markdown
from frappe.website.utils import get_shade, abs_url
...
...
...
# load jenv_filters from hooks.py
for app in frappe.get_installed_apps():
for jenv_filter in (frappe.get_hooks(app_name=app).jenv_filter or []):
filter_name, filter_function = jenv_filter.split(":")
jenv.filters[filter_name] = frappe.get_attr(filter_function)
And the one in version 10.1.35 is:
....
def set_filters(jenv):
import frappe
from frappe.utils import global_date_format, cint, cstr, flt, markdown
from frappe.website.utils import get_shade, abs_url
...
...
...
# load jenv_filters from hooks.py
for app in frappe.get_installed_apps():
for jenv_filter in frappe.get_hooks(app_name=app).get('jenv', {"filters": []})["filters"]:
filter_name, filter_function = jenv_filter.split(":")
jenv.filters[filter_name] = frappe.get_attr(filter_function)
You can clearly see the difference in the way the loop is pulling the filer and function name pair.
I have not tried in 10.1.35, but what I assume is that :
Following is best was to define filter in 10.1.29:
### hooks.py
jenv_filters = [
filter_name1:function_name1,
...
]
whereas this could be the way to try it out in 10.1.35:
### hooks.py
jenv = {
filters : [
filter_name1:function_name1,
...
]
}
Seems @sharath_c has pushed the change in the for loop in /frappe/frappe/utils/jinja.py in 10.1.35 before 2 months which was written by Anand 4 years ago. Can @sharath_c or anyone from the foundation can suggest the change which has been done here? Can you validate the above ?
Regards,
Parth Joshi
1 Like
@joshiparthin: Please refer this PR.
The previous hooks only had support for custom filters in jinja environment. Support for custom methods was added and hence changes to hooks declaration was done to keep all jinja environment hooks under one head.
3 Likes
@sharath_c This makes sense to me and I think the argument for consistency is a strong one. Thanks for providing the context here.
@joshiparthin Nice sleuthing, Parth.
1 Like