Skip to content

Conversation

@elfkuzco
Copy link
Collaborator

Rationale

By adding the offliner definition version to the schedule history entry, we can see the changes to the version recorded in the history.

Changes

  • add column to keep track of offliner definition updates in schedule history

This closes #1539

@elfkuzco elfkuzco requested a review from benoit74 November 25, 2025 09:22
@elfkuzco elfkuzco self-assigned this Nov 25, 2025
@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.38%. Comparing base (b0943ec) to head (2f56b64).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1540   +/-   ##
=======================================
  Coverage   83.37%   83.38%           
=======================================
  Files          91       91           
  Lines        4392     4394    +2     
  Branches      468      468           
=======================================
+ Hits         3662     3664    +2     
  Misses        608      608           
  Partials      122      122           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@elfkuzco elfkuzco requested a review from benoit74 November 26, 2025 09:00
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

Doing the update of latest existing history entry in a single SQL statement would be better (faster to execute since purely DB, probably less risk of edge cases).

Please confirm, but I feel like this would do the trick:

UPDATE schedule_history sh
SET offliner_definition_version = od.version
FROM schedule s
JOIN offliner_definition od
    ON s.offliner_definition_id = od.id
JOIN (
    SELECT DISTINCT ON (schedule_id)
        id AS history_id, schedule_id
    FROM schedule_history
    ORDER BY schedule_id, created_at DESC
) latest
    ON latest.history_id = sh.id
WHERE sh.schedule_id = s.id;

@elfkuzco
Copy link
Collaborator Author

It didn't work because the subquery re-selects from schedule_history and it seems PostgreSQL doesn't allow referencing the update target from inside the FROM clause when that table also appears in a subquery inside the FROM.

zf_backend     | sqlalchemy.exc.ProgrammingError: (psycopg.errors.UndefinedTable) invalid reference to FROM-clause entry for table "sh"
zf_backend     | LINE 13:                 ON latest.history_id = sh.id
zf_backend     |                                                 ^
zf_backend     | DETAIL:  There is an entry for table "sh", but it cannot be referenced from this part of the query.
zf_backend     | [SQL:
zf_backend     |             UPDATE schedule_history sh
zf_backend     |             SET offliner_definition_version = od.version
zf_backend     |             FROM schedule s
zf_backend     |             JOIN offliner_definition od
zf_backend     |                 ON s.offliner_definition_id = od.id
zf_backend     |             JOIN (
zf_backend     |                 SELECT DISTINCT ON (schedule_id)
zf_backend     |                     id AS history_id, schedule_id
zf_backend     |                 FROM schedule_history
zf_backend     |                 ORDER BY schedule_id, created_at DESC
zf_backend     |             ) latest
zf_backend     |                 ON latest.history_id = sh.id
zf_backend     |             WHERE sh.schedule_id = s.id;
zf_backend     |             ]

Fixed it by using a CTE instead

@elfkuzco elfkuzco requested a review from benoit74 November 27, 2025 08:51
Copy link
Collaborator

@benoit74 benoit74 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@elfkuzco elfkuzco force-pushed the offliner-definition-in-schedule-history branch from 8069e9e to 2f56b64 Compare November 27, 2025 08:57
@elfkuzco elfkuzco merged commit 37509ca into main Nov 27, 2025
5 checks passed
@elfkuzco elfkuzco deleted the offliner-definition-in-schedule-history branch November 27, 2025 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

offliner definition version is not tracked in schedule history

3 participants