Performance collection trigger after storage registration #506
Conversation
Codecov Report
@@ Coverage Diff @@
## master #506 +/- ##
==========================================
+ Coverage 69.06% 69.89% +0.83%
==========================================
Files 122 120 -2
Lines 9415 9239 -176
Branches 1055 1032 -23
==========================================
- Hits 6502 6458 -44
+ Misses 2567 2440 -127
+ Partials 346 341 -5
|
AmitRoushan
force-pushed
the
performance-dev
branch
4 times, most recently
from
6e07377
to
c1be218
Compare
| @mock.patch('delfin.common.config.load_json_file') | ||
| def test_delete(self, mock_load_json_file): | ||
| req = fakes.HTTPRequest.blank('/storages/fake_id') | ||
| mock_load_json_file.return_value = fake_schedular_config |
There was a problem hiding this comment.
code with scheduler conf is removed. testcase ```test_delete_invalid_schedular_conf```` is deleted
| @@ -139,11 +139,6 @@ def main(): | |||
| 'delfin', 'delfin.conf') | |||
| copy_files(conf_file_src, conf_file) | |||
|
|
|||
| # Copy the scheduler_config.json file | |||
There was a problem hiding this comment.
Need to remove installer reference , opened #507 in delfin and sodafoundation/installer#426 in installer
There was a problem hiding this comment.
Sure, The current code removal is part of scheduler conf related removal.
delfin/api/v1/storages.py
Outdated
| capabilities) | ||
| except Exception as e: | ||
| # Unexpected error occurred, while performance monitoring. | ||
| msg = _('Failed to trigger performance monitoring for storage: ' |
There was a problem hiding this comment.
how about changing message like 'Failed to create performance monitoring task for this storage '
|
|
||
|
|
||
| class Task(object): | ||
| DEFAULT_TASK_INTERVAL = 30 |
There was a problem hiding this comment.
Currently the value fetched from Constants for the completion of flow. Agree to move in conf file.
Can take it in upcoming PR of scheduler. Need to add all other configuration for scheduler, and scheduled task.
| task = dict() | ||
| task.update(storage_id=storage_id) | ||
| task.update(args=capabilities.get('resource_metrics')) | ||
| task.update(interval=constants.Task.DEFAULT_TASK_INTERVAL) |
There was a problem hiding this comment.
hope this has to be inn sync with scheduler , better to take from conf file.
There was a problem hiding this comment.
Yes agreed, That's why added as constant. Will fix it in scheduler PR
delfin/common/config.py
Outdated
| import socket | ||
| from delfin import exception | ||
|
|
||
| from apscheduler.schedulers.background import BackgroundScheduler |
delfin/common/config.py
Outdated
| LOG.error(e) | ||
| raise exception.ConfigNotFound(e) | ||
|
|
||
|
|
||
| class Scheduler: |
AmitRoushan
force-pushed
the
performance-dev
branch
from
c1be218
to
80ad942
Compare
AmitRoushan
force-pushed
the
performance-dev
branch
from
80ad942
to
d422a2a
Compare
sushanthakumar
force-pushed
the
performance-dev
branch
from
e07c01f
to
1731e49
Compare
sushanthakumar
force-pushed
the
performance-dev
branch
from
1731e49
to
2fcb938
Compare
What this PR does / why we need it:
This PR addresses performance collection trigger after storage registration
It removes the old performance collection implementation and respective files as the trigger and mechanism are changed
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Release note: