Skip to content

[chores] Misc clean up: CI gecko logs and autodiscovery of manage_estimated_locations task#1276

Merged
nemesifier merged 2 commits intomasterfrom
misc-fixes
Mar 14, 2026
Merged

[chores] Misc clean up: CI gecko logs and autodiscovery of manage_estimated_locations task#1276
nemesifier merged 2 commits intomasterfrom
misc-fixes

Conversation

@nemesifier
Copy link
Copy Markdown
Member

@nemesifier nemesifier commented Mar 14, 2026

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • N/A I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • N/A I have updated the documentation.

Description of Changes

  • Avoid false positive failures CI gecko log job
  • Automate discovery of manage_estimated_locations celery task

@nemesifier nemesifier self-assigned this Mar 14, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f7c57d39-7775-4fcd-9253-3de0b38a8b07

📥 Commits

Reviewing files that changed from the base of the PR and between bd7a80a and c2e9a59.

📒 Files selected for processing (1)
  • openwisp_controller/geo/tasks.py
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Python==3.12 | django~=5.1.0
  • GitHub Check: Python==3.11 | django~=5.2.0
  • GitHub Check: Python==3.12 | django~=5.2.0
  • GitHub Check: Python==3.11 | django~=4.2.0
  • GitHub Check: Python==3.12 | django~=4.2.0
  • GitHub Check: Python==3.10 | django~=4.2.0
  • GitHub Check: Python==3.11 | django~=5.1.0
  • GitHub Check: Python==3.10 | django~=5.2.0
  • GitHub Check: Python==3.10 | django~=5.1.0
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-15T14:06:53.460Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/base/models.py:63-93
Timestamp: 2026-01-15T14:06:53.460Z
Learning: In openwisp_controller/geo/base/models.py, the BaseLocation.save() method protects the is_estimated field from manual changes when the estimated location feature is disabled by reverting it to its initial database value using the elif branch. This is intentional behavior to ensure the field can only be modified when the feature is properly configured.

Applied to files:

  • openwisp_controller/geo/tasks.py
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.

Applied to files:

  • openwisp_controller/geo/tasks.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.

Applied to files:

  • openwisp_controller/geo/tasks.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.

Applied to files:

  • openwisp_controller/geo/tasks.py
🔇 Additional comments (1)
openwisp_controller/geo/tasks.py (1)

1-3: Autodiscovery bridge import is correct.

Line 1 intentionally re-exports manage_estimated_locations in openwisp_controller/geo/tasks.py, which ensures Celery task registration from openwisp_controller/geo/estimated_location/tasks.py during task autodiscovery. Keeping # noqa: F401 here is appropriate.


📝 Walkthrough

Walkthrough

The CI workflow step that printed geckodriver.log was changed to a shell snippet that checks for the file and prints it if present or emits a placeholder message if absent, avoiding errors when the log is missing. An explicit import was added to openwisp_controller/geo/tasks.py: from .estimated_location.tasks import manage_estimated_locations # noqa: F401 to support autodiscovery.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly indicates the type [chores], describes the main changes accurately (CI gecko logs and autodiscovery task), and aligns with the actual modifications in the changeset.
Description check ✅ Passed The description includes completed checklist items, a clear description of changes, and references specific issues, though the PR description template items are selectively marked as N/A rather than fully addressed.
Bug Fixes ✅ Passed PR contains infrastructure improvements to CI workflow and Celery task autodiscovery, not bug fixes to core user-facing functionality; CI changes fall under valid exception for Github Actions workflow fixes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch misc-fixes
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nemesifier nemesifier merged commit f3c99c4 into master Mar 14, 2026
26 of 30 checks passed
@nemesifier nemesifier deleted the misc-fixes branch March 14, 2026 19:34
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 98.658% (-0.01%) from 98.672%
when pulling c2e9a59 on misc-fixes
into d203490 on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants