Skip to content

feat: enhance refresh job scheduling with improved privilege handling… #551

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

Merged
jirhiker merged 2 commits into from
Feb 27, 2026
Merged

feat: enhance refresh job scheduling with improved privilege handling… #551

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
43852fe
feat: enhance refresh job scheduling with improved privilege handling…
jirhiker Feb 27, 2026
cf51d48
feat: handle internal error in pg_cron job unscheduling for better ro…
jirhiker Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 38 additions & 22 deletions alembic/versions/d5e6f7a8b9c0_create_pygeoapi_supporting_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,22 +234,37 @@ def _create_refresh_function() -> str:
def _schedule_refresh_job() -> str:
return f"""
DO $do$
Comment on lines 234 to 236
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The PR description is still the template (Why/How/Notes bullets aren’t filled in). Please update the description to summarize the motivation and approach so reviewers/operators can understand the intended behavior changes around pg_cron privilege/error handling.

Copilot uses AI. Check for mistakes.
DECLARE
existing_job_id bigint;
BEGIN
SELECT jobid INTO existing_job_id
FROM cron.job
WHERE jobname = '{REFRESH_JOB_NAME}';

IF existing_job_id IS NOT NULL THEN
PERFORM cron.unschedule(existing_job_id);
END IF;
BEGIN
-- Avoid direct SELECT on cron.job because managed Postgres
-- environments may deny access to the cron schema table.
PERFORM cron.unschedule('{REFRESH_JOB_NAME}');
EXCEPTION
WHEN undefined_function THEN
NULL;
WHEN invalid_parameter_value THEN
NULL;
WHEN internal_error THEN
-- Some pg_cron builds raise internal_error when the named
-- job does not exist. Treat this as already-unscheduled.
NULL;
WHEN insufficient_privilege THEN
RAISE NOTICE
'Skipping pg_cron unschedule for % due to insufficient privileges.',
'{REFRESH_JOB_NAME}';
RETURN;
END;

PERFORM cron.schedule(
Comment on lines +238 to 258
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

In _schedule_refresh_job, the undefined_function branch for cron.unschedule('{REFRESH_JOB_NAME}') is swallowed and then the code proceeds to cron.schedule(...). In environments where cron.unschedule(text) is not available, this means any pre-existing job with the same name will not be removed by this migration, which can lead to inconsistent behavior when scheduling (e.g., an existing job remains unchanged). Consider either (a) falling back to the jobid-based unschedule when possible, or (b) skipping scheduling when unschedule cannot run, or (c) handling the “job already exists” case when calling cron.schedule.

Copilot uses AI. Check for mistakes.
'{REFRESH_JOB_NAME}',
'{REFRESH_SCHEDULE}',
$cmd$SELECT public.{REFRESH_FUNCTION_NAME}();$cmd$
);
EXCEPTION
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

cron.schedule(...) is only wrapped with an EXCEPTION WHEN insufficient_privilege handler. If scheduling fails for other pg_cron-related reasons that this migration is already trying to be resilient to (e.g., undefined_function when pg_cron functions aren’t available, or other server-state errors), the migration will still abort here. Consider extending the exception handling (or adding a pre-check) so the migration can safely continue when pg_cron scheduling isn’t possible.

Suggested change
EXCEPTION
EXCEPTION
WHEN undefined_function THEN
RAISE NOTICE
'Skipping pg_cron schedule for % because cron functions are not available.',
'{REFRESH_JOB_NAME}';
WHEN invalid_parameter_value THEN
RAISE NOTICE
'Skipping pg_cron schedule for % due to invalid parameter value.',
'{REFRESH_JOB_NAME}';
WHEN internal_error THEN
RAISE NOTICE
'Skipping pg_cron schedule for % due to internal pg_cron error.',
'{REFRESH_JOB_NAME}';

Copilot uses AI. Check for mistakes.
WHEN insufficient_privilege THEN
RAISE NOTICE
'Skipping pg_cron schedule for % due to insufficient privileges.',
'{REFRESH_JOB_NAME}';
END
$do$;
"""
Expand All @@ -258,20 +273,21 @@ def _schedule_refresh_job() -> str:
def _unschedule_refresh_job() -> str:
return f"""
DO $do$
DECLARE
existing_job_id bigint;
BEGIN
IF to_regclass('cron.job') IS NULL THEN
RETURN;
END IF;

SELECT jobid INTO existing_job_id
FROM cron.job
WHERE jobname = '{REFRESH_JOB_NAME}';

IF existing_job_id IS NOT NULL THEN
PERFORM cron.unschedule(existing_job_id);
END IF;
BEGIN
PERFORM cron.unschedule('{REFRESH_JOB_NAME}');
EXCEPTION
WHEN undefined_function THEN
NULL;
WHEN invalid_parameter_value THEN
NULL;
WHEN internal_error THEN
NULL;
WHEN insufficient_privilege THEN
RAISE NOTICE
'Skipping pg_cron unschedule for % due to insufficient privileges.',
'{REFRESH_JOB_NAME}';
Comment on lines +280 to +283
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 27, 2026

Choose a reason for hiding this comment

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

P2 Badge Abort downgrade when pg_cron unschedule is unauthorized

Swallowing insufficient_privilege in the downgrade unschedule block allows the migration to continue and later drop public.refresh_pygeoapi_materialized_views(), even when the cron job could not be removed. In environments where the job is owned by a different role, this leaves the job active and it will keep firing a command that now points to a dropped function, causing recurring pg_cron execution errors after downgrade; previously this path failed fast instead of committing a broken partial teardown.

Useful? React with 👍 / 👎.

END;
END
$do$;
"""
Expand Down