Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork88
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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
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 initiativegetattr 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 ofprometheus_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.
| __title__="prometheus-connect" | ||
| __version__="0.5.7" | ||
| from .prometheus_connectimport*# 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 PrometheusApiClientExceptionthis 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 |
TMVKasiViswanath commentedFeb 22, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@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 toinit.py. Once the server is back up, rerunning the workflow should make the tests pass without any issues. |
@TMVKasiViswanath that should be fixed now. Could you please rebase and try again? |
Optimized imports in theinit 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.