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.

Sign up for GitHub

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

Jump to bottom

fix: update schedule disabler configs #1414

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix: update schedule disabler configs #1414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
31b692b
fix: update schedule disabler configs
rchandnaeg Feb 22, 2024
929b595
Merge pull request #1 from rchandnaeg/fix/schedule-disabler-configs
rchandnaeg Feb 22, 2024
76a7f59
fix: pre-commit prettier
rchandnaeg Feb 22, 2024
8f43609
Merge branch 'pinterest:master' into master
rchandnaeg Mar 1, 2024
9381913
Merge branch 'pinterest:master' into master
rchandnaeg Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 26 additions & 7 deletions querybook/server/tasks/disable_scheduled_docs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def get_scheduled_datadoc_tasks(session=None):
.filter(
TaskSchedule.enabled.is_(True),
TaskSchedule.name.like(DATADOC_SCHEDULE_PREFIX + "%"),
TaskSchedule.task_type == ScheduleTaskType.USER,
TaskSchedule.task_type == ScheduleTaskType.USER.value,
)
.all()
)
Expand Down Expand Up @@ -117,12 +117,12 @@ def check_task_no_impression_for_n_days(
):
doc_id = task_dict["kwargs"]["doc_id"]

if not disable_config.skip_if_no_impression_with_export:
if disable_config.skip_if_no_impression_with_export:
task_have_export = len(task_dict["kwargs"].get("exports", [])) > 0
if task_have_export:
return False

if not disable_config.skip_if_no_impression_but_non_select:
if disable_config.skip_if_no_impression_but_non_select:
Comment on lines +120 to +125
Copy link
Collaborator

Choose a reason for hiding this comment

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

@czgu could you help to look at this? I'm a bit confused about the actual meaning of these two configs. should it disable or not when they are true.
skip_if_no_impression_with_export
skip_if_no_impression_but_non_select

# Check if every DML/DDL of queries are select
doc = get_data_doc_by_id(doc_id, session=session)
queries_in_doc = [
Expand Down Expand Up @@ -220,8 +220,8 @@ def disable_deactivated_scheduled_docs(
disable_if_inactive_owner=True,
disable_if_failed_for_n_runs=5,
disable_if_no_impression_for_n_days=30,
skip_if_no_impression_but_non_select=False,
skip_if_no_impression_with_export=False,
skip_if_no_impression_but_non_select=True,
skip_if_no_impression_with_export=True,
),
session=None,
):
Expand Down Expand Up @@ -271,8 +271,27 @@ def disable_deactivated_scheduled_docs(

@celery.task(bind=True)
@with_task_logging()
def disable_scheduled_docs(self):
tasks_to_disable = disable_deactivated_scheduled_docs()
def disable_scheduled_docs(
self,
dry_run=False,
notifier=None,
disable_if_inactive_owner=True,
disable_if_failed_for_n_runs=5,
disable_if_no_impression_for_n_days=30,
skip_if_no_impression_but_non_select=True,
skip_if_no_impression_with_export=True,
):
tasks_to_disable = disable_deactivated_scheduled_docs(
dry_run=dry_run,
notifier=notifier,
disable_config=DisableConfig(
disable_if_inactive_owner=disable_if_inactive_owner,
disable_if_failed_for_n_runs=disable_if_failed_for_n_runs,
disable_if_no_impression_for_n_days=disable_if_no_impression_for_n_days,
skip_if_no_impression_but_non_select=skip_if_no_impression_but_non_select,
skip_if_no_impression_with_export=skip_if_no_impression_with_export,
),
)
if len(tasks_to_disable) == 0:
logger.info("No scheduled docs disabled.")
else:
Expand Down
9 changes: 8 additions & 1 deletion querybook/webapp/components/Task/TaskEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ function stringToTypedVal(stringVal: boolean | number | string) {
return false;
} else if (!isNaN(Number(stringVal))) {
return Number(stringVal);
} else if (
stringVal === 'null' ||
stringVal === 'undefined' ||
stringVal === 'None'
) {
return null;
} else {
return stringVal;
}
Expand Down Expand Up @@ -237,7 +243,8 @@ export const TaskEditor: React.FunctionComponent<IProps> = ({
);

const getKwargPlaceholder = (param: string) =>
registeredTaskParamList?.[values.task]?.[param] ?? 'Insert value';
String(registeredTaskParamList?.[values.task]?.[param]) ??
'Insert value';

const kwargsDOM = (
<div className="TaskEditor-kwargs mt12">
Expand Down