Script: "Add Django EOL checker script (comment if <180 days to EOL)"#8068
Script: "Add Django EOL checker script (comment if <180 days to EOL)"#8068CarolineDenis wants to merge 30 commits into
Conversation
Dependency EOL WarningsOne or more dependencies are approaching or past End-of-Life. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check_django_eol.py`:
- Around line 32-36: Wrap the requests.get(url) call with a try/except to catch
requests.exceptions.RequestException and add a reasonable timeout (e.g.
timeout=10); on exception log/print the exception message and call sys.exit(1).
Replace the plain requests.get(url) with something like a guarded call (try:
response = requests.get(url, timeout=10) except RequestException as e:
print(f"ERROR: Failed to fetch EOL data: {e}") ; sys.exit(1)), and keep the
existing status_code check for non-200 responses to exit deterministically.
- Around line 22-25: The current extraction that builds cycle from full_version
using ".".join(full_version.split(".")[:2]) fails for prerelease strings like
"5.2rc1"; update the logic that computes cycle (the variable named cycle derived
from full_version) to use a regex that captures only the numeric major and minor
components (e.g. r"^(\d+)\.(\d+)" or similar) and set cycle to "major.minor"
from those captures; replace the split-based assignment with this regex-based
extraction and handle the case where the pattern doesn't match by falling back
or raising a clear error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b802bf6-d259-481b-93c5-1aa5376189b9
📒 Files selected for processing (1)
scripts/check_django_eol.py
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Failed to clone repository into sandbox. Please try again. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 337-339: The workflow is installing a fresh Django via the `pip
install requests django` step which ignores the project's pinned dependencies;
replace that Install dependencies step so it installs the project's requirements
(e.g., use the project's requirements file or lockfile) instead of installing
`django` directly so the job checks the actual project Django version used at
runtime (update the step that runs `pip install requests django` to install from
the repo's pinned requirements such as `pip install -r requirements.txt` or
equivalent).
In `@scripts/check_django_eol.py`:
- Around line 12-16: The subprocess.run call that invokes Django version in
scripts/check_django_eol.py lacks a timeout and can hang CI; add a timeout
(e.g., timeout=10) to the subprocess.run call and handle
subprocess.TimeoutExpired around that call: on timeout log an error/print a
helpful message and exit non‑zero (mirroring existing error handling) so the
script fails fast instead of hanging. Reference the existing subprocess.run
invocation and the surrounding result handling to implement the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 606266e1-3ef5-4f9a-8c02-f03040129b12
📒 Files selected for processing (2)
.github/workflows/test.ymlscripts/check_django_eol.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/check_django_eol.py (1)
12-17:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix syntax error: missing comma after
text=True.Line 15 is missing a comma, which will cause a
SyntaxErrorwhen the script is executed. Python requires commas between function arguments.🔧 Proposed fix
result = subprocess.run( [sys.executable, "-m", "django", "--version"], capture_output=True, - text=True + text=True, timeout=30 )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check_django_eol.py` around lines 12 - 17, The subprocess.run call assigned to result has a missing comma between arguments (after text=True), causing a SyntaxError; edit the subprocess.run invocation (the line creating result) to add the missing comma so the arguments become valid (i.e., ensure text=True, timeout=30 is comma-separated) while leaving the rest of the call intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 364-382: Replace the multiline template literal used to build the
issue comment body (the const body that embeds `${output}`) with a safe
join-based construction to avoid nested backticks and interpolation issues:
construct an array of lines that includes a literal opening code fence, the
sanitized output variable (sanitize steps.eol.outputs.output by replacing
backticks and `${` sequences), the closing code fence, and the message lines,
then call .join('\n') to produce body; update the code around the const output /
body variables that feed github.rest.issues.createComment to use this new body
construction.
---
Duplicate comments:
In `@scripts/check_django_eol.py`:
- Around line 12-17: The subprocess.run call assigned to result has a missing
comma between arguments (after text=True), causing a SyntaxError; edit the
subprocess.run invocation (the line creating result) to add the missing comma so
the arguments become valid (i.e., ensure text=True, timeout=30 is
comma-separated) while leaving the rest of the call intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 38b44413-7c6a-48db-96c4-2c71438b4fda
📒 Files selected for processing (2)
.github/workflows/test.ymlscripts/check_django_eol.py
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
This dependency is approaching or past End-of-Life. |
This django version is approaching or past End-of-Life. |
Dependency EOL WarningsOne or more dependencies are approaching or past End-of-Life. |
|
NOTES:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
scripts/eol/runtimes.py (1)
1-5: ⚡ Quick winConsider using dataclass for cleaner, more Pythonic code.
The
Runtimeclass is a simple data container. Using@dataclasswould reduce boilerplate, add automatic__repr__, and make the intent clearer.♻️ Proposed refactor
+from dataclasses import dataclass + +@dataclass class Runtime: - def __init__(self, name, api, display_name): - self.name = name - self.api = api - self.display_name = display_name + name: str + api: str + display_name: str🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/eol/runtimes.py` around lines 1 - 5, Replace the boilerplate Runtime class with a dataclass: add "from dataclasses import dataclass" and annotate the class with `@dataclass`, converting the explicit __init__ and attribute assignments into type-annotated fields (e.g., name: str, api: str, display_name: str) so Runtime becomes a concise data container with autogenerated __init__, __repr__, and equality helpers; keep the class name Runtime and the same attribute names to preserve existing usages.scripts/eol/runner.py (1)
12-24: 💤 Low valueClarify filter logic when product is not detected.
When
filter_productis specified but not detected, line 16 creates a dictionary entry with aNonevalue that is immediately skipped on line 19. While functional, explicitly handling the missing product would be clearer.♻️ Proposed refactor
def run_all(filter_product=None): detected = scan_repo() if filter_product: - detected = {filter_product: detected.get(filter_product)} + if filter_product in detected: + detected = {filter_product: detected[filter_product]} + else: + print(f"Product '{filter_product}' not detected in repository") + return for name, version in detected.items(): - if not version: - continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/eol/runner.py` around lines 12 - 24, The filter logic in run_all currently builds detected = {filter_product: detected.get(filter_product)} which inserts a None entry when the product isn't found and then skips it; change run_all to explicitly check for presence after calling scan_repo (use scan_repo() result) and if filter_product is provided but not in the detected keys, handle it clearly—either return early or log/raise a clear message—rather than inserting a None value; update references to detected, filter_product, RUNTIMES and runtime handling accordingly so the flow is explicit when the product is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 320-354: The eol-check job currently writes to $GITHUB_OUTPUT in
the step with id "eol" but never exposes that data as a job output nor uploads
an artifact, while the eol-comment job expects to download artifacts; fix by
either (A) exposing the check output as a job output: capture the script output
in the step with id "eol" and write a named output (e.g. echo
"eol_output=$output" >> $GITHUB_OUTPUT), add a job-level outputs: eol_output:
${{ steps.eol.outputs.eol_output }}, and consume that in eol-comment via
needs.eol-check.outputs.eol_output, or (B) upload a real artifact from eol-check
using actions/upload-artifact (write the script output to a file from
scripts.eol.check_eol step and call actions/upload-artifact) and then change
eol-comment to download that artifact with actions/download-artifact; choose one
approach and update the "eol-check" step (id "eol" / script
scripts.eol.check_eol) and the "eol-comment" job accordingly.
In `@scripts/eol/check_eol.py`:
- Line 10: The import of run_one is using the wrong module path: change the
import in scripts/eol/check_eol.py that currently references eol.runner to
import run_one from scripts.eol.runner so it matches other imports (e.g., the
module uses package-relative imports) and avoids ImportError when invoking the
script; update the line that imports run_one accordingly to reference
scripts.eol.runner and keep the symbol name run_one unchanged.
In `@scripts/eol/eol_api.py`:
- Around line 23-26: The calculate_days_remaining function must handle
non-string and boolean false EOL values and invalid date formats; update
calculate_days_remaining to first check if eol_date is a str (and not a boolean
False), and if not (or if it's falsy/False) return a sentinel (e.g., None) for
"no EOL"; then wrap the datetime.strptime call in a try/except to catch
ValueError and return the same sentinel (or log the parse error) instead of
allowing an exception to propagate. Ensure you reference
calculate_days_remaining when making these changes and keep the return type
consistent (Optional[int]) across callers.
In `@scripts/eol/scanner.py`:
- Around line 26-29: The detect_python function currently returns a hardcoded
"3.12"; change it to actually detect the repo Python version by inspecting
common sources instead: update detect_python to first try parsing pyproject.toml
for project.requires-python (use tomli or safe parsing and extract a major.minor
with a regex), then check .python-version and runtime.txt for a numeric
major.minor, and finally scan GitHub Actions workflow files (e.g.,
.github/workflows/*) for a setup-python python-version entry; return the first
matched "X.Y" string or None if nothing found and handle parse errors
gracefully.
- Around line 6-23: The detect_node function currently only checks a root
Path("package.json") and catches all exceptions; change it to search for
package.json files recursively (e.g., using Path().rglob("package.json") or
specifically include the nested path "specifyweb/frontend/js_src/package.json"),
inspect each file's parsed JSON for the "engines" -> "node" field, extract the
major version with your existing regex logic and return the first valid match;
replace the broad except Exception with narrow exception handlers such as
json.JSONDecodeError and OSError (and optionally ValueError) around json.loads
and file reads so only expected file/parse errors are suppressed while other
errors surface.
---
Nitpick comments:
In `@scripts/eol/runner.py`:
- Around line 12-24: The filter logic in run_all currently builds detected =
{filter_product: detected.get(filter_product)} which inserts a None entry when
the product isn't found and then skips it; change run_all to explicitly check
for presence after calling scan_repo (use scan_repo() result) and if
filter_product is provided but not in the detected keys, handle it
clearly—either return early or log/raise a clear message—rather than inserting a
None value; update references to detected, filter_product, RUNTIMES and runtime
handling accordingly so the flow is explicit when the product is missing.
In `@scripts/eol/runtimes.py`:
- Around line 1-5: Replace the boilerplate Runtime class with a dataclass: add
"from dataclasses import dataclass" and annotate the class with `@dataclass`,
converting the explicit __init__ and attribute assignments into type-annotated
fields (e.g., name: str, api: str, display_name: str) so Runtime becomes a
concise data container with autogenerated __init__, __repr__, and equality
helpers; keep the class name Runtime and the same attribute names to preserve
existing usages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e126462a-940c-49d0-9341-d76e1ce69c5b
📒 Files selected for processing (8)
.github/workflows/test.ymlscripts/__init__.pyscripts/eol/__init__.pyscripts/eol/check_eol.pyscripts/eol/eol_api.pyscripts/eol/runner.pyscripts/eol/runtimes.pyscripts/eol/scanner.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Connect Timeout Error (attempted address: api.github.com:443, timeout: 10000ms) |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 4 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 4 unresolved review comments. Co-authored-by: CodeRabbit <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
scripts/eol/runner.py (2)
34-38: 💤 Low valueRemove duplicate VERSION and CYCLE output.
Lines 35 and 36 print
{name.upper()}_VERSIONand{name.upper()}_CYCLEwith identical values. In this implementation, the version is already normalized to match the cycle format, making the duplication unnecessary and potentially confusing.🧹 Simplification
print(f"STATUS={status}") print(f"{name.upper()}_VERSION={version}") - print(f"{name.upper()}_CYCLE={version}") print(f"EOL_DATE={eol_date}") print(f"DAYS_REMAINING={days}")If consumers need the cycle separately in the future, it should be passed as a distinct parameter rather than duplicated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/eol/runner.py` around lines 34 - 38, The two consecutive print statements emitting the same value for {name.upper()}_VERSION and {name.upper()}_CYCLE are redundant; remove the duplicate output by deleting the print that emits "{name.upper()}_CYCLE={version}" (or alternatively keep CYCLE and remove VERSION) so only one of those keys is printed, leaving the other prints (STATUS, EOL_DATE, DAYS_REMAINING) unchanged; if a distinct cycle value is needed later, accept a separate parameter instead of duplicating the version.
12-24: ⚡ Quick winConsider adding feedback when filtered product is not detected.
When
run_one()is called with a product name that isn't detected in the repository (line 16 creates{filter_product: None}), the loop silently skips it at line 19 with no output. This could confuse users who expect some indication that the requested product wasn't found.💡 Suggested improvement
if filter_product: detected = {filter_product: detected.get(filter_product)} + if not detected.get(filter_product): + print(f"Product '{filter_product}' not detected in repository") + return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/eol/runner.py` around lines 12 - 24, run_all currently creates a {filter_product: None} entry when a filtered product isn't found and then silently skips it; update run_all to detect this case and emit a clear user-facing message (e.g., via print/processLogger/CLI logger) when filter_product is not present in the scan_repo() results (use detected.get(filter_product) or "filter_product in detected" check) before iterating, then either return early or continue so users know the requested product wasn't detected; reference run_all, scan_repo, detected and RUNTIMES to locate the change.scripts/eol/check_eol.py (1)
1-11: 💤 Low valueMove run_one import to top level for conventional style.
The conditional import of
run_oneon line 10 is unconventional. Since bothrun_allandrun_oneare from the same module, there's no performance benefit to lazy loading, and moving the import to the top would improve readability.♻️ Suggested refactor
import sys -from scripts.eol.runner import run_all +from scripts.eol.runner import run_all, run_one if __name__ == "__main__": if len(sys.argv) == 1: run_all() else: - from scripts.eol.runner import run_one run_one(sys.argv[1].lower())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/eol/check_eol.py` around lines 1 - 11, The conditional import of run_one inside main should be moved to the top-level alongside run_all for clarity; import both run_all and run_one from scripts.eol.runner at module top, remove the inner "from scripts.eol.runner import run_one" line, and keep the conditional logic that calls run_all() when no args else calls run_one(sys.argv[1].lower()) so behavior is unchanged while improving readability.scripts/eol/runtimes.py (1)
1-5: ⚡ Quick winConsider using a dataclass with type hints for better maintainability.
The
Runtimeclass would benefit from type annotations and could be simplified with@dataclass, which would automatically generate__init__,__repr__, and__eq__methods.♻️ Proposed refactor
+from dataclasses import dataclass + +@dataclass class Runtime: - def __init__(self, name, api, display_name): - self.name = name - self.api = api - self.display_name = display_name + name: str + api: str + display_name: str🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/eol/runtimes.py` around lines 1 - 5, Replace the simple Runtime class with a dataclass including type hints: import dataclasses and annotate the attributes name: str, api: str (or the appropriate type), and display_name: str, then add the `@dataclass` decorator above the Runtime class so __init__, __repr__, and __eq__ are generated automatically (ensure to remove the manual __init__ and update any references if needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 320-362: The eol-check job is suppressing failures so CI never
fails: remove the error suppression by deleting the continue-on-error: true on
the eol-check job and remove the "|| true" fallback in the Run EOL check step so
the exit code from scripts.eol.check_eol propagates; if you want the job to
actually block merges on warnings, also update runner.py (the script that runs
the check) to exit with a non-zero status when it reports WARNING (so the
workflow sees failure).
In `@scripts/eol/eol_api.py`:
- Around line 7-20: The get_eol_date function currently compares
release["cycle"] to the cycle parameter which can mismatch types; cast/normalize
release["cycle"] to a string (or otherwise normalize both sides to the same
type) before comparison so integer cycles from the API match scanner-returned
string cycles; update the comparison in get_eol_date (the loop over
response.json() and the if release["cycle"] == cycle check) to compare
normalized strings (e.g., str(release["cycle"]) == cycle) and keep existing
error handling intact.
In `@scripts/eol/scanner.py`:
- Around line 86-92: In detect_django(), wrap the call to req_file.read_text()
in a try/except to handle OSError (or more generally Exception) from unreadable
files; on exception log or emit a clear diagnostic and return None (or otherwise
propagate a controlled error) instead of letting the exception bubble up—this
change should touch the detect_django function and the req_file =
Path("requirements.txt")/content variable usage so subsequent logic only runs
when content was successfully read.
- Around line 94-98: The regex in scanner.py that assigns to match currently
allows prefixes like "django-extensions"; update the pattern used in re.search
(the r"(?i)^django\s*([<>=!~]=?)\s*([\d\.]+)" expression) to require a word
boundary after "django" so it only matches the exact package name (e.g., change
to use "\b" after "django"), keeping the rest of the groups ([<>=!~]=?) and
([\d\.]+) unchanged.
---
Nitpick comments:
In `@scripts/eol/check_eol.py`:
- Around line 1-11: The conditional import of run_one inside main should be
moved to the top-level alongside run_all for clarity; import both run_all and
run_one from scripts.eol.runner at module top, remove the inner "from
scripts.eol.runner import run_one" line, and keep the conditional logic that
calls run_all() when no args else calls run_one(sys.argv[1].lower()) so behavior
is unchanged while improving readability.
In `@scripts/eol/runner.py`:
- Around line 34-38: The two consecutive print statements emitting the same
value for {name.upper()}_VERSION and {name.upper()}_CYCLE are redundant; remove
the duplicate output by deleting the print that emits
"{name.upper()}_CYCLE={version}" (or alternatively keep CYCLE and remove
VERSION) so only one of those keys is printed, leaving the other prints (STATUS,
EOL_DATE, DAYS_REMAINING) unchanged; if a distinct cycle value is needed later,
accept a separate parameter instead of duplicating the version.
- Around line 12-24: run_all currently creates a {filter_product: None} entry
when a filtered product isn't found and then silently skips it; update run_all
to detect this case and emit a clear user-facing message (e.g., via
print/processLogger/CLI logger) when filter_product is not present in the
scan_repo() results (use detected.get(filter_product) or "filter_product in
detected" check) before iterating, then either return early or continue so users
know the requested product wasn't detected; reference run_all, scan_repo,
detected and RUNTIMES to locate the change.
In `@scripts/eol/runtimes.py`:
- Around line 1-5: Replace the simple Runtime class with a dataclass including
type hints: import dataclasses and annotate the attributes name: str, api: str
(or the appropriate type), and display_name: str, then add the `@dataclass`
decorator above the Runtime class so __init__, __repr__, and __eq__ are
generated automatically (ensure to remove the manual __init__ and update any
references if needed).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a5bc0152-7e1e-4063-830c-0a2f06a304b8
📒 Files selected for processing (8)
.github/workflows/test.ymlscripts/__init__.pyscripts/eol/__init__.pyscripts/eol/check_eol.pyscripts/eol/eol_api.pyscripts/eol/runner.pyscripts/eol/runtimes.pyscripts/eol/scanner.py
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 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 |
Fixes #8063
Checklist
self-explanatory (or properly documented)
specify7/specifyweb/specify/management/commands/run_key_migration_functions.py
Line 50 in ea04665
Testing instructions
Summary by CodeRabbit
New Features
Chores