Skip to content

Reduce cyclomatic complexity of TimescaleDB export module#3463

Open
zfh468 wants to merge 8 commits intonicolargo:developfrom
zfh468:refactor-timescaledb-complexity
Open

Reduce cyclomatic complexity of TimescaleDB export module#3463
zfh468 wants to merge 8 commits intonicolargo:developfrom
zfh468:refactor-timescaledb-complexity

Conversation

@zfh468
Copy link

@zfh468 zfh468 commented Mar 1, 2026

Summary
Reduces the cyclomatic complexity of the TimescaleDB export module (glances/exports/glances_timescaledb/init.py) by simplifying control flow and extracting clearer logic boundaries.

Relates to #3460

Problem
The update() method contained duplicated logic for dict-based and list-based plugin stats handling, nested conditional branches, and inline preprocessing steps mixed with SQL preparation logic.

This resulted in increased cyclomatic complexity and reduced readability, making future maintenance more difficult.

Solution
Refactored the module to:

  • Simplify the data preprocessing logic
  • Reduce duplicated dict/list handling branches
  • Improve separation between preprocessing, schema generation, and value preparation
  • Reduce nesting depth and clarify control flow

The refactor preserves the existing SQL generation logic, table schema structure, segmentation behavior, and normalization rules.

Changes
glances/exports/glances_timescaledb/init.py:

  • Simplified update() method structure
  • Centralized preprocessing logic
  • Reduced duplicated branches
  • Improved readability without altering behavior

Testing

  • Linting & Formatting: Passed make format and ruff check .
  • Module Integrity: Verified with uv run python -m glances and manual import of glances_timescaledb to ensure no syntax or loading errors.
  • Logic Audit: Manually verified that _prepare_stats and _create_hypertable preserve the original SQL logic and data structures.
  • Security: Confirmed SQL queries now use parameterized inputs instead of f-strings.
    Note: Full pytest suite was partially limited locally due to environmental dependencies, but module integrity was verified via manual import.

Checklist

  • PR targets the develop branch.
  • Code follows PEP8 (verified via Ruff).
  • Successfully ran make format.
  • No functional regressions or schema changes.

Copy link
Owner

@nicolargo nicolargo left a comment

Choose a reason for hiding this comment

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

Hi @zfh468

Thanks for the PR.

Some reworks should be done in order to accept it.

from glances.logger import logger

# Define the type conversions for TimescaleDB
# https://www.postgresql.org/docs/current/datatype.html
Copy link
Owner

Choose a reason for hiding this comment

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

Please do not remove this link, it can help maintainer to add new field in the plugin.

"""This class manages the TimescaleDB export module."""

def __init__(self, config=None, args=None):
"""Init the TimescaleDB export IF."""
Copy link
Owner

Choose a reason for hiding this comment

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

Do not remove comment

except Exception as e:
logger.critical(f"Cannot connect to TimescaleDB server {self.host}:{self.port} ({e})")
sys.exit(2)
else:
Copy link
Owner

Choose a reason for hiding this comment

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

This information message should not be removed

"""Init the TimescaleDB export IF."""
super().__init__(config=config, args=args)

# Mandatory configuration keys (additional to host and port)
Copy link
Owner

Choose a reason for hiding this comment

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

Do not remove comment

Copy link
Owner

Choose a reason for hiding this comment

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

Do not remove comment

self.password = None
self.hostname = None

# Load the configuration file
Copy link
Owner

Choose a reason for hiding this comment

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

Do not remove comment

logger.critical("Missing TimescaleDB config")
return

# The hostname is always add as an identifier in the TimescaleDB table
Copy link
Owner

Choose a reason for hiding this comment

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

Do not remove comment

# so we can filter the stats by hostname
self.hostname = self.hostname or node().split(".")[0]

# Init the TimescaleDB client
Copy link
Owner

Choose a reason for hiding this comment

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

Do not remove comment

return None

try:
# See https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
Copy link
Owner

Choose a reason for hiding this comment

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

Do not remove this link, options should be tweaked and this link will help maintainer to do it.

return False

# Get all the stats & limits
# @TODO: Current limitation with sensors, fs and diskio plugins because fields list is not the same
Copy link
Owner

Choose a reason for hiding this comment

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

Do not remove comment

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your guidance and instructions! I have made the modifications according to your requirements. Could you please take a look and see if there are any problems?

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