Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
7 changes: 1 addition & 6 deletions openwisp_monitoring/check/base/models.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
from collections import OrderedDict

from django.contrib.contenttypes.fields import GenericForeignKey
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.utils.functional import cached_property
from django.utils.module_loading import import_string
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField

from openwisp_utils.base import TimeStampedEditableModel

Expand Down Expand Up @@ -37,13 +34,11 @@ class AbstractCheck(TimeStampedEditableModel):
db_index=True,
max_length=128,
)
params = JSONField(
params = models.JSONField(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same

_("parameters"),
default=dict,
blank=True,
help_text=_("parameters needed to perform the check"),
load_kwargs={"object_pairs_hook": OrderedDict},
dump_kwargs={"indent": 4},
)

class Meta:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django on 2026-02-23
# Replaces third-party jsonfield.fields.JSONField with Django's built-in models.JSONField

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("check", "0011_check_active_object_checks_idx"),
]

operations = [
migrations.AlterField(
model_name="check",
name="params",
field=models.JSONField(
blank=True,
default=dict,
help_text="parameters needed to perform the check",
verbose_name="parameters",
),
),
]
9 changes: 2 additions & 7 deletions openwisp_monitoring/monitoring/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from django.db import IntegrityError, models
from django.utils import timezone
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from openwisp_notifications.signals import notify
from pytz import timezone as tz
from pytz import utc
Expand Down Expand Up @@ -91,20 +90,16 @@ class AbstractMetric(TimeStampedEditableModel):
)
object_id = models.CharField(max_length=36, db_index=True, blank=True, null=True)
content_object = GenericForeignKey("content_type", "object_id")
main_tags = JSONField(
main_tags = models.JSONField(
Copy link
Copy Markdown

@Eeshu-Yadav Eeshu-Yadav Apr 2, 2026

Choose a reason for hiding this comment

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

Please add encoder=DjangoJSONEncoder to all JSONField instances (this one, extra_tags, and params in check/base/models.py).
This is needed to prevent serialization errors with lazy translation objects. See openwisp/openwisp-notifications#438 and the reference commit: openwisp/openwisp-notifications@bad5232

@nemesifier requested this same change on both the controller PR (openwisp/openwisp-controller#1214) and the radius PR (openwisp/openwisp-radius#619). It should be applied consistently across all openwisp repos.

from django.core.serializers.json import DjangoJSONEncoder

main_tags = models.JSONField(
    _("main tags"),
    default=dict,
    blank=True,
    db_index=True,
    encoder=DjangoJSONEncoder,
)

The AlterField migrations (0012, 0013, and the test migrations) will also need encoder=django.core.serializers.json.DjangoJSONEncoder in their field definitions.

_("main tags"),
default=dict,
blank=True,
load_kwargs={"object_pairs_hook": OrderedDict},
dump_kwargs={"indent": 4},
db_index=True,
)
extra_tags = JSONField(
extra_tags = models.JSONField(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same

_("extra tags"),
default=dict,
blank=True,
load_kwargs={"object_pairs_hook": OrderedDict},
dump_kwargs={"indent": 4},
)
Comment on lines +93 to 103
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for any remaining imports of the third-party jsonfield package
echo "=== Remaining jsonfield imports ==="
rg -n 'from jsonfield' --type=py
rg -n 'import jsonfield' --type=py

echo ""
echo "=== jsonfield in dependency files ==="
rg -n 'jsonfield' -g 'setup.*' -g 'pyproject.toml' -g 'requirements*' -g 'Pipfile*'

Repository: openwisp/openwisp-monitoring

Length of output: 527


Remove jsonfield imports from migration files or restore package as a dependency.

The original jsonfield package is no longer listed in project dependencies (setup.py/setup.cfg/pyproject.toml/requirements*.txt), but it remains imported in migration files:

  • openwisp_monitoring/monitoring/migrations/0004_metric_main_and_extra_tags.py
  • openwisp_monitoring/check/migrations/0001_initial_squashed_0002_check_unique_together.py
  • tests/openwisp2/sample_check/migrations/0001_initial.py
  • tests/openwisp2/sample_monitoring/migrations/0001_initial.py

These migrations will fail when executed because the jsonfield package is unavailable. Either update the migration files to remove jsonfield usage or restore the package as a dependency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@openwisp_monitoring/monitoring/base/models.py` around lines 93 - 103,
Migrations reference the external jsonfield package which is no longer a project
dependency; update the listed migration files (the Migration classes in
0004_metric_main_and_extra_tags,
0001_initial_squashed_0002_check_unique_together, tests.../0001_initial.py
files) to remove jsonfield imports and use Django's built-in JSONField instead
(e.g., import JSONField from django.db.models or reference models.JSONField for
fields like main_tags and extra_tags), ensuring the field definitions and
deconstruction in those Migration classes use JSONField so migrations run
without adding jsonfield as a dependency.

# NULL means the health has yet to be assessed
is_healthy = models.BooleanField(default=None, null=True, blank=True, db_index=True)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django on 2026-02-23
# Replaces third-party jsonfield.fields.JSONField with Django's built-in models.JSONField

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("monitoring", "0012_migrate_signal_metrics"),
]

operations = [
migrations.AlterField(
model_name="metric",
name="main_tags",
field=models.JSONField(
blank=True,
db_index=True,
default=dict,
verbose_name="main tags",
),
),
migrations.AlterField(
model_name="metric",
name="extra_tags",
field=models.JSONField(
blank=True,
default=dict,
verbose_name="extra tags",
),
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django on 2026-02-23
# Replaces third-party jsonfield.fields.JSONField with Django's built-in models.JSONField

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("sample_check", "0003_add_check_inline_permissions"),
]

operations = [
migrations.AlterField(
model_name="check",
name="params",
field=models.JSONField(
blank=True,
default=dict,
help_text="parameters needed to perform the check",
verbose_name="parameters",
),
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django on 2026-02-23
# Replaces third-party jsonfield.fields.JSONField with Django's built-in models.JSONField

from django.db import migrations, models

class Migration(migrations.Migration):
dependencies = [
("sample_monitoring", "0004_alter_metric_field_name"),
]

operations = [
migrations.AlterField(
model_name="metric",
name="main_tags",
field=models.JSONField(
blank=True,
db_index=True,
default=dict,
verbose_name="main tags",
),
Comment on lines +16 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

db_index=True on JSONField creates a B-tree index, not GIN

On PostgreSQL, db_index=True on a jsonb column produces a B-tree index on the binary JSON representation. If main_tags is queried using key containment or key existence operators (@>, ?), a GIN index would be significantly more effective. This mirrors the original jsonfield behavior and is not a regression, but worth revisiting if monitoring queries filter by individual tag keys.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@tests/openwisp2/sample_monitoring/migrations/0005_replace_jsonfield_with_django_builtin.py`
around lines 16 - 21, The migration sets db_index=True on the JSONField
`main_tags` (models.JSONField), which creates a B-tree index; change this to use
a GIN index instead by removing db_index=True from the JSONField and adding an
Index entry using django.contrib.postgres.indexes.GinIndex for the `main_tags`
column (e.g. add a migrations.AddIndex that references
GinIndex(fields=['main_tags']) in the same migration so queries using
containment/existence operators use a proper GIN index).

),
migrations.AlterField(
model_name="metric",
name="extra_tags",
field=models.JSONField(
blank=True,
default=dict,
verbose_name="extra tags",
),
),
]
Loading