Skip to content

Commit

Permalink
Merge branch 'develop' into workflow-builder
Browse files Browse the repository at this point in the history
  • Loading branch information
shariquerik committed May 10, 2023
2 parents ecc4318 + 90d03d9 commit 93f9492
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 43 deletions.
8 changes: 5 additions & 3 deletions frappe/core/doctype/data_import/data_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from frappe.core.doctype.data_import.importer import Importer
from frappe.model.document import Document
from frappe.modules.import_file import import_file_by_path
from frappe.utils.background_jobs import enqueue, is_job_queued
from frappe.utils.background_jobs import enqueue, is_job_enqueued
from frappe.utils.csvutils import validate_google_sheets_url


Expand Down Expand Up @@ -64,13 +64,15 @@ def start_import(self):
if is_scheduler_inactive() and not frappe.flags.in_test:
frappe.throw(_("Scheduler is inactive. Cannot import data."), title=_("Scheduler Inactive"))

if not is_job_queued(self.name):
job_id = f"data_import::{self.name}"

if not is_job_enqueued(job_id):
enqueue(
start_import,
queue="default",
timeout=10000,
event="data_import",
job_name=self.name,
job_id=job_id,
data_import=self.name,
now=frappe.conf.developer_mode or frappe.flags.in_test,
)
Expand Down
22 changes: 18 additions & 4 deletions frappe/core/doctype/report/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,18 @@ def execute_script(self, filters):
return self.get_columns(), loc["result"]

def get_data(
self, filters=None, limit=None, user=None, as_dict=False, ignore_prepared_report=False
self,
filters=None,
limit=None,
user=None,
as_dict=False,
ignore_prepared_report=False,
are_default_filters=True,
):
if self.report_type in ("Query Report", "Script Report", "Custom Report"):
columns, result = self.run_query_report(filters, user, ignore_prepared_report)
columns, result = self.run_query_report(
filters, user, ignore_prepared_report, are_default_filters
)
else:
columns, result = self.run_standard_report(filters, limit, user)

Expand All @@ -169,10 +177,16 @@ def get_data(

return columns, result

def run_query_report(self, filters=None, user=None, ignore_prepared_report=False):
def run_query_report(
self, filters=None, user=None, ignore_prepared_report=False, are_default_filters=True
):
columns, result = [], []
data = frappe.desk.query_report.run(
self.name, filters=filters, user=user, ignore_prepared_report=ignore_prepared_report
self.name,
filters=filters,
user=user,
ignore_prepared_report=ignore_prepared_report,
are_default_filters=are_default_filters,
)

for d in data.get("columns"):
Expand Down
18 changes: 4 additions & 14 deletions frappe/core/doctype/rq_job/test_rq_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from frappe.core.doctype.rq_job.rq_job import RQJob, remove_failed_jobs, stop_job
from frappe.tests.utils import FrappeTestCase, timeout
from frappe.utils import cstr, execute_in_shell
from frappe.utils.background_jobs import is_job_enqueued, is_job_queued
from frappe.utils.background_jobs import is_job_enqueued


class TestRQJob(FrappeTestCase):
Expand Down Expand Up @@ -87,17 +87,6 @@ def test_delete_doc(self):
with self.assertRaises(rq_exc.NoSuchJobError):
job.refresh()

def test_is_enqueued(self):

dummy_job = frappe.enqueue(self.BG_JOB, sleep=10, queue="short")
job_name = "uniq_test_job"
actual_job = frappe.enqueue(self.BG_JOB, job_name=job_name, queue="short")

self.assertTrue(is_job_queued(job_name))
stop_job(dummy_job.id)
self.check_status(actual_job, "finished")
self.assertFalse(is_job_queued(job_name))

@timeout(20)
def test_multi_queue_burst_consumption(self):
for _ in range(3):
Expand All @@ -110,9 +99,10 @@ def test_multi_queue_burst_consumption(self):
@timeout(20)
def test_job_id_dedup(self):
job_id = "test_dedup"
job = frappe.enqueue(self.BG_JOB, sleep=10, job_id=job_id)
job = frappe.enqueue(self.BG_JOB, sleep=5, job_id=job_id)
self.assertTrue(is_job_enqueued(job_id))
stop_job(job.id)
self.check_status(job, "finished")
self.assertFalse(is_job_enqueued(job_id))


def test_func(fail=False, sleep=0):
Expand Down
5 changes: 5 additions & 0 deletions frappe/core/doctype/scheduled_job_type/scheduled_job_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ def enqueue(self, force=False) -> bool:
job_id=self.rq_job_id,
)
return True
else:
frappe.logger("scheduler").error(
f"Skipped queueing {self.method} because it was found in queue for {frappe.local.site}"
)

return False

def is_event_due(self, current_time=None):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def get_report_content(self):
filters=self.filters,
as_dict=True,
ignore_prepared_report=True,
are_default_filters=False,
)

# add serial numbers
Expand Down
2 changes: 1 addition & 1 deletion frappe/public/js/frappe/form/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ frappe.ui.form.save = function (frm, action, callback, btn) {

const is_empty_row = function (cells) {
for (let i = 0; i < cells.length; i++) {
if (locals[doc.doctype][doc.name][cells[i].fieldname]) {
if (locals[doc.doctype][doc.name] && locals[doc.doctype][doc.name][cells[i].fieldname]) {
return false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion frappe/public/js/frappe/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ $.extend(frappe.model, {
get_all_docs: function (doc) {
var all = [doc];
for (var key in doc) {
if ($.isArray(doc[key])) {
if ($.isArray(doc[key]) && !key.startsWith("_")) {
var children = doc[key];
for (var i = 0, l = children.length; i < l; i++) {
all.push(children[i]);
Expand Down
27 changes: 7 additions & 20 deletions frappe/utils/background_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from frappe import _
from frappe.utils import cstr, get_bench_id
from frappe.utils.commands import log
from frappe.utils.deprecations import deprecation_warning
from frappe.utils.redis_queue import RedisQueue

if TYPE_CHECKING:
Expand Down Expand Up @@ -76,7 +77,7 @@ def enqueue(
:param timeout: should be set according to the functions
:param event: this is passed to enable clearing of jobs from queues
:param is_async: if is_async=False, the method is executed immediately, else via a worker
:param job_name: can be used to name an enqueue call, which can be used to prevent duplicate calls
:param job_name: [DEPRECATED] can be used to name an enqueue call, which can be used to prevent duplicate calls
:param now: if now=True, the method is executed via frappe.call
:param kwargs: keyword arguments to be passed to the method
:param job_id: Assigning unique job id, which can be checked using `is_job_enqueued`
Expand All @@ -88,11 +89,12 @@ def enqueue(
# namespace job ids to sites
job_id = create_job_id(job_id)

if job_name:
deprecation_warning("Using enqueue with `job_name` is deprecated, use `job_id` instead.")

if not is_async and not frappe.flags.in_test:
print(
_(
"Using enqueue with is_async=False outside of tests is not recommended, use now=True instead."
)
deprecation_warning(
"Using enqueue with is_async=False outside of tests is not recommended, use now=True instead."
)

call_directly = now or (not is_async and not frappe.flags.in_test)
Expand Down Expand Up @@ -430,21 +432,6 @@ def test_job(s):
time.sleep(s)


def is_job_queued(job_name: str) -> bool:
"""Check if job exists with given job_name
DEPRECATED: Use `job_id` parameter while enqueueing job instead instead.
"""
for queue in get_queues():
for job_id in queue.get_job_ids():
if not job_id:
continue
job = queue.fetch_job(job_id)
if job.kwargs.get("job_name") == job_name and job.kwargs.get("site") == frappe.local.site:
return True
return False


def create_job_id(job_id: str) -> str:
"""Generate unique job id for deduplication"""
return f"{frappe.local.site}::{job_id}"
Expand Down

0 comments on commit 93f9492

Please sign in to comment.