Registration POST API and Synch_all implementation framework #12
Conversation
| @@ -11,23 +11,28 @@ | |||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
|
|
|||
| from os_service_types import exc | |||
| else: | ||
| LOG.info("Stdout: {0}".format(stdout)) | ||
| return stdout, stderr | ||
|
|
There was a problem hiding this comment.
Is this a temporary implementation? If yes, Please mark/comment accordingly. This seems to be test placeholder
| def register(self): | ||
|
|
||
| # the real implementation | ||
| time.sleep(1) |
There was a problem hiding this comment.
Why is this sleep required here?
dolphin/resource_manager/storages.py
Outdated
|
|
||
| class DeviceModel: | ||
| def __init__(self): | ||
| self.id = None |
There was a problem hiding this comment.
Follow python style of initialization self.id=self.name
self.description = None
:) :)
dolphin/resource_manager/storages.py
Outdated
| device = DeviceModel() | ||
| device.id = uuidutils.generate_uuid() | ||
| device.created_at = timeutils.utcnow() | ||
| if body.get('name'): |
There was a problem hiding this comment.
body is dict..Now rather than going for if condition statements at multiple lines, will it be better to make it one liner as:
device.name = body.get('name', "")
So if you get the value fine, else it will assign the default which is "" here or you can keep it None
|
|
||
|
|
||
| def build_storage(storage): | ||
| view = copy.deepcopy(storage) |
There was a problem hiding this comment.
Can you put comment why deepcopy is required?
|
|
||
| driver = drivermanager.Driver() | ||
| device_info = driver.register() | ||
| if device_info.get('status') == 'available': |
There was a problem hiding this comment.
So we are clear that the device will be registered only if the status is available and no other state (!error)?
| driver = dm.Driver() | ||
| driver.list_volumes(context, request_spec) | ||
| except: | ||
| LOG.error("Volume retreival failed in driver") |
There was a problem hiding this comment.
log the exception print_trace at debug level...Will help in future
dolphin/task_manager/manager.py
Outdated
| :param req: | ||
| :return: | ||
| """ | ||
| LOG.info("Produce task, say hello ...") |
There was a problem hiding this comment.
Hello :)
remove this line
| try: | ||
| self.task_rpcapi.get_volumes(context, device) | ||
| except: | ||
| LOG.error('Volumes retreival failed!!') |
There was a problem hiding this comment.
| LOG.error('Volumes retreival failed!!') | |
| LOG.error('Volumes retrieval failed!!') |
|
Is it possible to give the test example? For example, what url is used for test, and what is the supposed respone? |
|
@sfzeng |
What this PR does / why we need it:
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: