Skip to content

Potential fix for 1 code quality finding #183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vizsatiz wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Potential fix for 1 code quality finding #183

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 15 additions & 19 deletions ...console/floconsole/db/alembic/versions/2025_12_13_1400-a1b2c3d4e5f6_create_default_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,23 @@ def upgrade() -> None:
if default_app_name and default_app_public_url and default_app_private_url:
# Check if a default app already exists with the same name
conn = op.get_bind()
# Insert default app
# Insert default app using SQLAlchemy Table reflection for better type safety
app_id = uuid.uuid4()
conn.execute(
sa.text("""
INSERT INTO app (id, app_name, public_url, private_url, deleted, status, config, deployment_type, type, created_at, updated_at)
VALUES (:id, :app_name, :public_url, :private_url, :deleted, :status, :config, :deployment_type, :type, :created_at, :updated_at)
"""),
{
'id': app_id,
'app_name': default_app_name,
'public_url': default_app_public_url,
'private_url': default_app_private_url,
'deleted': False,
'status': 'success',
'config': '{}',
'deployment_type': 'manual',
'type': 'custom',
'created_at': datetime.now(),
'updated_at': datetime.now(),
},
app_table = sa.Table('app', sa.MetaData(), autoload_with=conn)
insert_stmt = app_table.insert().values(
id=app_id,
app_name=default_app_name,
public_url=default_app_public_url,
private_url=default_app_private_url,
deleted=False,
status='success',
config='{}',
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In SQLAlchemy 2.x Core, when inserting into a JSON column via insert().values(config='{}'), does it store a JSON string ("{}") or a JSON object ({})? Please cite SQLAlchemy docs for bind/result JSON behavior.

💡 Result:

In SQLAlchemy 2.x Core, when inserting into a JSON column using insert.values(config='{}'), it stores a JSON string ("{}"). The JSON type has a bind processor that automatically serializes Python objects (dict, list, etc.) to JSON strings before sending to the database. When passing a string literal like '{}', it is sent as-is without further serialization, resulting in the JSON string "{}" being stored. SQLAlchemy documentation confirms JSON types use bind_processor and result_processor for serialization/deserialization. Multiple sources, including Stack Overflow discussions and GitHub, note that passing a pre-serialized string leads to it being stored directly, while Python dicts are automatically json.dumps-ed. For example, passing config='{}' binds the string "{}", stored as JSON string. Passing config={} serializes to "{}" via bind processor.

Citations:

🏁 Script executed:

cat -n wavefront/server/apps/floconsole/floconsole/db/alembic/versions/2025_12_13_1400-a1b2c3d4e5f6_create_default_app.py | head -60

Repository: rootflo/wavefront

Length of output: 2362

🏁 Script executed:

find . -type f -name "*.py" -path "*/alembic/versions/*" | head -20

Repository: rootflo/wavefront

Length of output: 2590

🏁 Script executed:

find . -type f -name "*.py" -path "*/alembic/*" | grep -E "(create|initial)" | head -10

Repository: rootflo/wavefront

Length of output: 1328

🏁 Script executed:

cat -n ./wavefront/server/apps/floconsole/floconsole/db/alembic/versions/2025_08_21_1251-ac10dc573599_create_app_table.py | head -80

Repository: rootflo/wavefront

Length of output: 1617

🏁 Script executed:

cat -n ./wavefront/server/apps/floconsole/floconsole/db/alembic/versions/2025_10_14_1514-e9a4691e0732_update_apps_table.py

Repository: rootflo/wavefront

Length of output: 1344

Use a Python dict for the config column value, not a JSON string.

The config column is defined as sa.JSON() in the schema. In SQLAlchemy, passing a string literal config='{}' causes the bind processor to send it as-is to the database, storing a JSON string ("{}") instead of a JSON object ({}). Pass a Python dict config={} instead, which SQLAlchemy's bind processor will correctly serialize.

Proposed fix 
-            config='{}',
+            config={},
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
config='{}',
config={},
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@wavefront/server/apps/floconsole/floconsole/db/alembic/versions/2025_12_13_1400-a1b2c3d4e5f6_create_default_app.py`
at line 45, The migration is inserting a JSON column value as a string literal
"{}", which stores a JSON string instead of an object; update the insert for the
config column so it uses a Python dict (config={}) rather than a string
(config='{}') in the Alembic revision (the
2025_12_13_1400-a1b2c3d4e5f6_create_default_app.py change) where the row/object
with key "config" is defined so SQLAlchemy's JSON bind processor serializes it
as a true JSON object.

deployment_type='manual',
type='custom',
created_at=datetime.now(),
updated_at=datetime.now(),
)
conn.execute(insert_stmt)


def downgrade() -> None:
Expand Down