Skip to content

Resolved issue #225 #293

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

Merged
4n4nd merged 2 commits into from
Dec 4, 2025
Merged

Resolved issue #225 #293

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88ae193
Resolved issue #225
TMVKasiViswanath Feb 21, 2025
4fdea05
Refactored __init__.py to improve imports and maintainability
TMVKasiViswanath Feb 21, 2025
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
23 changes: 18 additions & 5 deletions prometheus_api_client/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,21 @@
__title__ = "prometheus-connect"
__version__ = "0.5.7"

from .prometheus_connect import * # noqa F403
Copy link
Collaborator

@harshad16 harshad16 Feb 21, 2025

Choose a reason for hiding this comment

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

The additional classes also need to be adjusted, if we are explicitly defining each class available with prometheus_api_client.

from prometheus_api_client import PrometheusApiClientException

this would fails with current implementation.

Copy link
Contributor Author

@TMVKasiViswanath TMVKasiViswanath Feb 21, 2025

Choose a reason for hiding this comment

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

yes, I forgot to add 2 more imports in the init file

from .metric import Metric # noqa F401
from .metrics_list import MetricsList # noqa F401
from .metric_snapshot_df import MetricSnapshotDataFrame # noqa F401
from .metric_range_df import MetricRangeDataFrame # noqa F401
from .exceptions import PrometheusApiClientException, MetricValueConversionError
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Exception classes should also be lazy-loaded for consistency. Currently PrometheusApiClientException and MetricValueConversionError are eagerly imported on line 6, which defeats part of the purpose of this optimization. These exceptions should be added to the __getattr__ function like the other classes, and only imported when actually used.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Missing blank line between imports and function definition. PEP 8 style guide recommends two blank lines before top-level function definitions to improve readability.

Suggested change
from .exceptions import PrometheusApiClientException, MetricValueConversionError
from .exceptions import PrometheusApiClientException, MetricValueConversionError

Copilot uses AI. Check for mistakes.
def __getattr__(name):
if name == "PrometheusConnect":
from .prometheus_connect import PrometheusConnect
return PrometheusConnect
elif name == "Metric":
from .metric import Metric
return Metric
elif name == "MetricsList":
from .metrics_list import MetricsList
return MetricsList
elif name == "MetricSnapshotDataFrame":
from .metric_snapshot_df import MetricSnapshotDataFrame
return MetricSnapshotDataFrame
elif name == "MetricRangeDataFrame":
from .metric_range_df import MetricRangeDataFrame
return MetricRangeDataFrame
raise AttributeError(f"module {__name__} has no attribute {name}")
Comment on lines +7 to +23
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

Missing __all__ definition for lazy-loaded modules. When implementing lazy imports via __getattr__, it's a best practice to also define __all__ to explicitly declare the public API. This ensures tools like type checkers, IDEs, and from module import * work correctly. Add: __all__ = ["PrometheusConnect", "Metric", "MetricsList", "MetricSnapshotDataFrame", "MetricRangeDataFrame", "PrometheusApiClientException", "MetricValueConversionError"]

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The error message formatting uses an f-string but doesn't include quotes around the attribute name, making it inconsistent with Python's standard AttributeError format. The standard format is "module 'module_name' has no attribute 'attr_name'" (note the quotes around both module and attribute names). Change to: raise AttributeError(f"module '{__name__}' has no attribute '{name}'")

Suggested change
raise AttributeError(f"module {__name__} has no attribute {name}")
raise AttributeError(f"module '{__name__}' has no attribute '{name}'")

Copilot uses AI. Check for mistakes.
Loading