utils_misc: Reorg disk related APIs into utils_disk #2103

balamuruhans · 2019-06-04T07:17:07Z

Reorganise disk related method to utils_disk and break circular
imports caused with respect to disk related APIs in utils_misc and
gluster, move some of storage apis to utils_disk and utils_storage
for better work flow. Rename utils_disk.py to disk.py

Signed-off-by: Srikanth Aithal [email protected]
Signed-off-by: Balamuruhan S [email protected]

lgtm-com · 2019-06-04T07:47:42Z

This pull request introduces 12 alerts when merging 3c5ffd6 into 3852a5c - view on LGTM.com

new alerts:

12 for Module-level cyclic import

virttest/utils_disk.py

lgtm-com · 2019-06-05T14:54:42Z

This pull request introduces 1 alert when merging 3093a9d into 3852a5c - view on LGTM.com

new alerts:

1 for Module-level cyclic import

lgtm-com · 2019-06-07T09:59:41Z

This pull request introduces 1 alert when merging 0dee3e5 into cb277eb - view on LGTM.com

new alerts:

1 for Unused import

balamuruhans · 2019-06-07T11:38:47Z

@vivianQizhu I have worked to reorganize disk related methods to avoid circular imports and to cleanup utils_misc.py. I have moved most of your code in this reorganization work, I would request for your review and acknowledgement. Thanks!

lgtm-com · 2019-07-09T07:12:53Z

This pull request introduces 1 alert when merging 03fe125 into 509ca09 - view on LGTM.com

new alerts:

1 for Variable defined multiple times

balamuruhans · 2019-07-09T07:33:02Z

@luckyh request for your review. This is part of cleaning up utlils_misc.py, there is no logical changes I have moved the disk related APIs to organise it.

luckyh · 2019-07-10T08:36:28Z

@balamuruhans ok, I see there are several problems been solved in this PR, so let's put them into individual commits, thanks.

lgtm-com · 2019-08-19T09:49:27Z

This pull request introduces 2 alerts and fixes 1 when merging 07144ed into 5c83fe5 - view on LGTM.com

new alerts:

1 for Unused import
1 for Modification of parameter with default

fixed alerts:

1 for Modification of parameter with default

lgtm-com · 2019-08-19T11:06:28Z

This pull request introduces 1 alert and fixes 1 when merging 25afa10 into 5c83fe5 - view on LGTM.com

new alerts:

1 for Modification of parameter with default

fixed alerts:

1 for Modification of parameter with default

balamuruhans · 2019-08-19T12:08:40Z

@luckyh I have separated out commits and included @bssrikanth's patch work with this PR to avoid CI failure/checks. request for your review.

luckyh

@balamuruhans I like this improvement, but we should take care about these things for preventing it breaking our current and future testing, let's consider more about that, thanks!

virttest/gluster.py

virttest/storage.py

virttest/utils_disk.py

luckyh · 2019-08-20T05:49:12Z

virttest/utils_misc.py

-    if ostype == "windows":
-        return format_windows_disk(session, did, mountpoint, size, fstype)
-    return format_linux_disk(session, did, all_disks_did, partition,
-                             mountpoint, size, fstype)


We should take care of the above 5 functions for moving out them from this module since there are several test scripts using them currently.

@bssrikanth is working on some more disk related functions into utils_disk with #2148. I thought of getting in this first and he can rebase the PR #2148.

virttest/utils_storage.py

lgtm-com · 2019-08-20T09:20:35Z

This pull request introduces 1 alert when merging 8b35297 into 3f08daa - view on LGTM.com

new alerts:

1 for Module-level cyclic import

balamuruhans · 2019-08-20T10:12:43Z

@luckyh Thanks for reviewing, the reason that I forgot why I moved some APIs from gluster.py to utils_disk.py is to avoid the above cyclic import issue,
libvirt.py
libvirt_storage.py
storage.py
gluster.py
utils_disk.py
pool_xml.py
libvirt_storage.py

If the APIs file_exists() and glusterfs_mount() to utils_disk.py this can be fixed. If you think on any other solution please help me on it.

balamuruhans · 2019-08-21T07:01:32Z

@luckyh could you help to review the changes. Thanks!

selftests/unit/test_libvirt_xml.py

luckyh · 2019-08-26T08:51:23Z

virttest/gluster.py

@@ -204,80 +201,6 @@ def gluster_brick_delete(brick_path, session=None):
            logging.error("Not able to delete brick folder %s", details)


-@error_context.context_aware
-def gluster_vol_create(vol_name, hostname, brick_path, force=False, session=None):


If you just want to avoid circular imports, it's better to look for calling alternatives rather than move these things out

I could not get about this idea, can you help me to explain it ?

virttest/utils_disk.py

Reorganise disk related method to utils_disk and break circular imports caused with respect to disk related APIs in utils_misc and gluster to utils_disk for better work flow. Rename utils_disk.py to disk.py Signed-off-by: Srikanth Aithal <[email protected]> Signed-off-by: Balamuruhan S <[email protected]>

sathnaga

LGTM, minor, change commit heading to Reorg disk related APIs otherwords, avoid utils_disk from commit heading, as anyways it has been renamed to disk.py

balamuruhans force-pushed the utils_disk_org branch from 43de837 to 3c5ffd6 Compare June 4, 2019 07:18

balamuruhans changed the title ~~utils_disk: move new_disk_vol_name() to utils_disk~~ utils_test/libvirt: move new_disk_vol_name() to utils_disk Jun 4, 2019

balamuruhans mentioned this pull request Jun 4, 2019

virsh_vol_*: changes required to move disk related API autotest/tp-libvirt#2189

Closed

balamuruhans force-pushed the utils_disk_org branch from 3c5ffd6 to 01e1884 Compare June 4, 2019 09:26

Yingshun reviewed Jun 5, 2019

View reviewed changes

virttest/utils_disk.py Outdated Show resolved Hide resolved

balamuruhans force-pushed the utils_disk_org branch 3 times, most recently from 7a34ecb to 3093a9d Compare June 5, 2019 14:31

balamuruhans force-pushed the utils_disk_org branch from 3093a9d to 0dee3e5 Compare June 7, 2019 09:33

balamuruhans force-pushed the utils_disk_org branch from 0dee3e5 to 503680c Compare June 7, 2019 11:03

balamuruhans changed the title ~~utils_test/libvirt: move new_disk_vol_name() to utils_disk~~ utils_test/libvirt: Reorg disk related APIs into utils_disk Jun 7, 2019

balamuruhans mentioned this pull request Jun 26, 2019

Fix the FileNotFoundError of get_model_features() #2052

Merged

balamuruhans force-pushed the utils_disk_org branch from 503680c to 03fe125 Compare July 9, 2019 06:49

balamuruhans force-pushed the utils_disk_org branch from 03fe125 to 34b4607 Compare July 9, 2019 07:15

balamuruhans force-pushed the utils_disk_org branch from 34b4607 to 07144ed Compare August 19, 2019 09:20

balamuruhans force-pushed the utils_disk_org branch from 07144ed to 25afa10 Compare August 19, 2019 10:39

balamuruhans changed the title ~~utils_test/libvirt: Reorg disk related APIs into utils_disk~~ utils_misc: Reorg disk related APIs into utils_disk Aug 19, 2019

luckyh requested changes Aug 20, 2019

View reviewed changes

balamuruhans force-pushed the utils_disk_org branch from 25afa10 to 8b35297 Compare August 20, 2019 08:52

balamuruhans force-pushed the utils_disk_org branch from 8b35297 to a4058a7 Compare August 21, 2019 06:26

luckyh requested changes Aug 26, 2019

View reviewed changes

balamuruhans force-pushed the utils_disk_org branch 2 times, most recently from 5170a81 to 609f52f Compare August 28, 2019 11:45

balamuruhans force-pushed the utils_disk_org branch from 609f52f to a1d965b Compare August 29, 2019 06:43

sathnaga approved these changes Aug 29, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

utils_misc: Reorg disk related APIs into utils_disk #2103

utils_misc: Reorg disk related APIs into utils_disk #2103

balamuruhans commented Jun 4, 2019 •

edited

Loading

lgtm-com bot commented Jun 4, 2019

lgtm-com bot commented Jun 5, 2019

lgtm-com bot commented Jun 7, 2019

balamuruhans commented Jun 7, 2019

lgtm-com bot commented Jul 9, 2019

balamuruhans commented Jul 9, 2019

luckyh commented Jul 10, 2019

lgtm-com bot commented Aug 19, 2019

lgtm-com bot commented Aug 19, 2019

balamuruhans commented Aug 19, 2019

luckyh left a comment

luckyh Aug 20, 2019

balamuruhans Aug 20, 2019

lgtm-com bot commented Aug 20, 2019

balamuruhans commented Aug 20, 2019 •

edited

Loading

balamuruhans commented Aug 21, 2019

luckyh Aug 26, 2019

balamuruhans Aug 26, 2019

sathnaga left a comment

utils_misc: Reorg disk related APIs into utils_disk #2103

Are you sure you want to change the base?

utils_misc: Reorg disk related APIs into utils_disk #2103

Conversation

balamuruhans commented Jun 4, 2019 • edited Loading

lgtm-com bot commented Jun 4, 2019

lgtm-com bot commented Jun 5, 2019

lgtm-com bot commented Jun 7, 2019

balamuruhans commented Jun 7, 2019

lgtm-com bot commented Jul 9, 2019

balamuruhans commented Jul 9, 2019

luckyh commented Jul 10, 2019

lgtm-com bot commented Aug 19, 2019

lgtm-com bot commented Aug 19, 2019

balamuruhans commented Aug 19, 2019

luckyh left a comment

Choose a reason for hiding this comment

luckyh Aug 20, 2019

Choose a reason for hiding this comment

balamuruhans Aug 20, 2019

Choose a reason for hiding this comment

lgtm-com bot commented Aug 20, 2019

balamuruhans commented Aug 20, 2019 • edited Loading

balamuruhans commented Aug 21, 2019

luckyh Aug 26, 2019

Choose a reason for hiding this comment

balamuruhans Aug 26, 2019

Choose a reason for hiding this comment

sathnaga left a comment

Choose a reason for hiding this comment

balamuruhans commented Jun 4, 2019 •

edited

Loading

balamuruhans commented Aug 20, 2019 •

edited

Loading