Enforce endDate >= startDate and serialize Pages deployments - #81
Open
sparkyfen wants to merge 4 commits into
Open
Enforce endDate >= startDate and serialize Pages deployments#81sparkyfen wants to merge 4 commits into
sparkyfen wants to merge 4 commits into
Conversation
The schema expressed this as formatMinimum with a $data pointer, an ajv extension that python-jsonschema silently ignores, so events with an end date before their start date passed CI (ainmhicon 2027 in PR #77 shipped with endDate 2026-04-04 against startDate 2027-04-02 and validated green). Enforce the constraint explicitly in materialize.py's event loop and fix the adjacent has_errors flag, which was never set, so schema-invalid files now skip event processing instead of risking KeyErrors. Tested by .github/scripts/test_materialize_dates.py, which runs the real script against fixture series files and is wired into validate.yml.
…idation - Drop the accidentally committed .pyc and add a .gitignore so compiled bytecode can't be tracked again. - Make test_schema_invalid_file_still_fails actually discriminate: the pre-fix code also exited 1 (via an uncaught KeyError), so the test now asserts the ErrorLogger message and the absence of a traceback. Verified by mutation: reverting the has_errors fix fails the test. - Validate imported data in import_fancons, import_rams and import_furrynz before their push to main, mirroring import_concat, so a bad imported date fails the import instead of breaking deploy. - Document the ajv-only formatMinimum keyword in schema.json and the preconditions of the string date comparison in materialize.py.
- Mirror import_concat.yml's failure-annotation steps in the three importer workflows so a red run says which stage failed, and shrink the duplicated rationale comment to one line. - Decouple the schema-invalid test from jsonschema's exact message wording, run materialize via uv run --script, and cover the case where a later (non-first) event in a series has the bad dates. - Condense the date-check comment in materialize.py.
- deploy.yml gets the standard Pages concurrency group (one deployment at a time, queued intermediates skipped, in-progress never cancelled; same pattern as actions/starter-workflows pages/static.yml). Merging four PRs back to back on 2026-08-12 raced the Pages API and two runs failed with 'deployment request failed due to in progress deployment'. - Compare event dates as parsed values instead of raw strings, so the check no longer depends on the format checker admitting only canonical YYYY-MM-DD spellings. - Skip the materialize tests with a clear message when uv is not on PATH instead of tracebacking.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe materializer now enforces event date ordering and records schema failures per series. New integration tests cover these cases. Import workflows validate formatted data before commits. Pages deployments now use concurrency control. ChangesMaterialization validation
Mergeability Score: ⚪ Minimal · up to This PR adds date validation, import validation, and serialized Pages deployments; no actionable merge-blocking risk remains beyond normal checks and review. Deployment concurrency
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ImportWorkflow
participant Formatter
participant Materialize
participant CommitStep
ImportWorkflow->>Formatter: Format imported JSON
Formatter-->>ImportWorkflow: Formatting status
ImportWorkflow->>Materialize: Materialize and schema-validate data
Materialize-->>ImportWorkflow: Validation status
ImportWorkflow->>CommitStep: Commit only when checks pass
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
endDate >= startDateintools/materialize.py. The schema has always declared this viaformatMinimum/$data, but that's an ajv extension that python-jsonschema silently ignores, so it was never enforced — Updated "Ainmhicon" to include 2027 event #77 shipped a 2027 event with a 2026 end date and validation passed. Dates are compared as parsed values, so the check doesn't depend on which ISO spellings the format checker admits.has_errorsflag. Schema-invalid series files used to fall through into the event loop; they're now skipped. This also stops the event loop from writing output files for series whose ids never passed the schema's pattern check.import_fancons,import_rams, andimport_furrynzbefore their push to main, mirroringimport_concatincluding its failure annotations — a bad imported date now fails the import run instead of breaking every deploy.concurrency: pagesgroup indeploy.yml(same pattern as actions/starter-workflowspages/static.yml). Merging four PRs back to back on 2026-08-12 raced the Pages API: two deploy runs failed with "deployment request failed due to in progress deployment"..gitignore(__pycache__/,*.pyc).Tests
.github/scripts/test_materialize_dates.py(wired intovalidate.yml) runs the real materialize script against fixture series files: order violation, same-day event, violation on a non-first event, and a schema-invalid file that must be reported without a traceback. Each test fails when its guarded change is reverted. All 226 existing series files pass the new check, so this lands green with no data fixes needed.Rollout
Nothing manual: validation and deploy changes ride the existing workflows on merge. Recommend squash-merge — an accidentally committed
.pycexists in this branch's first commit and squashing keeps it out of main's history.Summary by CodeRabbit