-
-
Notifications
You must be signed in to change notification settings - Fork 80
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.
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
base: master
Are you sure you want to change the base?
Resolved issue #225 #293
Conversation
@TMVKasiViswanath all tests seem to be failing. Could you give it a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great initiative
getattr
in init.py might increase the maintainability of the module , right ?
I'm also not able to follow, how this fixes the #225 request.
as the request there is to, split the module with main dependency and optional dependency.
The current installation of prometheus_api_client
bring in pandas and additional package that is only required for Metrics class usage but not for PrometheusConnect operation.
For fixing the issue, we would need to change the packaging of the module.
"""A collection of tools to collect and manipulate prometheus metrics.""" | ||
|
||
__title__ = "prometheus-connect" | ||
__version__ = "0.5.7" | ||
|
||
from .prometheus_connect import * # noqa F403 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Yes I have made the changes and made the new commit to it |
@4n4nd The test failures are not caused by my changes. The errors indicate that demo.robustperception.io:9090 is unreachable ([Errno 101] Network is unreachable). To verify, I set up a local Prometheus instance, pointed PrometheusConnect to it, and ran the tests. Everything passed successfully.(All the 44 test cases in the tests folder got executed) This suggests that the issue is with the availability of the demo server, not my modifications to init.py. Once the server is back up, rerunning the workflow should make the tests pass without any issues. |
Optimized imports in the init method by implementing lazy imports. This prevents unnecessary imports of unused classes, ensuring that only the required classes are imported when needed. This reduces memory usage and improves efficiency. I have tested this to confirm that the changes work as expected.