-
Notifications
You must be signed in to change notification settings - Fork 8
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
Implement an interface from fetching job metrics from InfluxDB #663
Implement an interface from fetching job metrics from InfluxDB #663
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/PENG-2455--job-metric-collection #663 +/- ##
============================================================================
+ Coverage 92.57% 92.68% +0.10%
============================================================================
Files 85 87 +2
Lines 4483 4632 +149
============================================================================
+ Hits 4150 4293 +143
- Misses 333 339 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…fluxDB and send it to the API
01fb385
to
71d3ffd
Compare
msgpack = "^1.1.0" | ||
influxdb = "^5.3.2" |
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.
I think these two guys could be optional dependencies. On initialize_influx_client
, unsure to cover ImportError
if metrics are enabled but dependencies are not installed.
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.
that would require the logistical part of setting up the agent to install these dependencies. I think that, for running the agent easily, it's better to leave as is
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.
I had no auto installation in mind, at installation time they could be included with pip install jobbergate-agent[metrics]
, for instance. But at the same time it is not a big deal, it is more a suggestion.
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.
I wonder how that would be customizable in the snap. I'd had to spend a bit of time on that
…umba instead of vanilla Python. As well as, create a module specific for compute related operations.
…ding the metrics to the API
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.
Nice work, here. Added some comments for you!
…a structure expected by the API
…ergate_agent/jobbergate/schemas.py*
…_success* unit test
179f0ec
into
feature/PENG-2455--job-metric-collection
What
This PR implement the necessary code in the Jobbergate Agent and its Snap to enable job metric collection from InfluxDB.
The implementation involves a settings parameter which is used for connecting to InfluxDB. The logic happens during the
active_submissions_task
task. If any part of the upload of metrics of an active job submission fails, it won't interrupt the execution.Once fetched the metrics for an active job submission, the agent sends to the API.
Why
Provide insights regarding jobs through the API.
Task
: https://sharing.clickup.com/t/h/c/18022949/PENG-2457/BU7UOA63B936N27Peer Review
Please follow the upstream omnivector documentation concerning
peer-review guidelines.