3:58 p.m.
Sergey Gotliv has uploaded a new change for review.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
getFileList() filters files in case incensitive manner
Currently Engine is using getIsoList() and getFloppyList() APIs to
retrieve the list of iso and floppy files respectively. Both these APIs
are working in case incensitive manner but don't provide file
statistics. Changing getFileList(), which is already providing full
information about files, to be case incensitive will make it adoption
easier.
Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Signed-off-by: Sergey Gotliv <sgotliv(a)redhat.com>
---
M vdsm/storage/hsm.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/76/20476/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 4579763..e0423c4 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -2245,7 +2245,7 @@
dom = sdCache.produce(sdUUID=sdUUID)
if not dom.isISO or dom.getStorageType() != sd.NFS_DOMAIN:
raise se.GetFileListError(sdUUID)
- filesDict = dom.getFileList(pattern=pattern, caseSensitive=True)
+ filesDict = dom.getFileList(pattern=pattern, caseSensitive=False)
return {'files': filesDict}
@public
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
3:46 a.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Replace unused getFileList() with more flexible getFileStats()
1. Rename to getFileStats().
New name getFileStats() better describes the essence of that API.
This is safe to rename it because this API is not in use.
2. Add caseSensitive option.
Currently Engine is using getIsoList() and getFloppyList() APIs to
retrieve the list of iso and floppy files respectively. Both these APIs
are working in case-insensitive manner but don't provide file
statistics. getFileStats() returns file statistics. Adding
caseSensitive option allows using getFileStats() instead of
getIsoList() and getFloppyList().
Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Bug-Url: https://bugzilla.redhat.com/1005889
Signed-off-by: Sergey Gotliv <sgotliv(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/20476
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
Reviewed-by: Ayal Baron <abaron(a)redhat.com>
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/BindingXMLRPC.py
M vdsm/storage/hsm.py
M vdsm/storage/storage_exception.py
M vdsm_api/Bridge.py
M vdsm_api/vdsmapi-schema.json
7 files changed, 38 insertions(+), 30 deletions(-)
Approvals:
Ayal Baron: Looks good to me, approved
Nir Soffer: Looks good to me, but someone else must approve
Federico Simoncelli: Looks good to me, but someone else must approve
Sergey Gotliv: Verified
Dan Kenigsberg: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3:46 a.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Dan Kenigsberg has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
6:02 p.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Sergey Gotliv has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
5:58 a.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Nir Soffer has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:21 a.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5129/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4325/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5203/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3:31 a.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Sergey Gotliv has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 5:
Yes, it's correct. I did a lot of changes due to your comments in the patch set 5. See
diff between 5 and 4.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:56 p.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Sergey Gotliv has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 5:
(1 comment)
....................................................
File vdsm/storage/hsm.py
Line 2238: :param caseSensitive: Enables case-sensitive matching.
Line 2239: :type caseSensitive: bool
Line 2240: :options: ?
Line 2241:
Line 2242: :returns: a dict of all files and statistics matching the pattern.
Done
Line 2243: :rtype: dict
Line 2244: """
Line 2245: vars.task.setDefaultException(se.GetFileStatsError(sdUUID))
Line 2246: vars.task.getSharedLock(STORAGE, sdUUID)
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
2:29 p.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Nir Soffer has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
Nice - there is one possible documentation cleanup if you post another version.
....................................................
File vdsm/storage/hsm.py
Line 2238: :param caseSensitive: Enables case-sensitive matching.
Line 2239: :type caseSensitive: bool
Line 2240: :options: ?
Line 2241:
Line 2242: :returns: a dict of all files and statistics matching the pattern.
:returns: file statistics for files matching pattern.
Is more correct. The fact that we return a dict is specified in :rtype: bellow.
Line 2243: :rtype: dict
Line 2244: """
Line 2245: vars.task.setDefaultException(se.GetFileStatsError(sdUUID))
Line 2246: vars.task.getSharedLock(STORAGE, sdUUID)
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
11:52 a.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5101/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4297/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5175/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:28 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
Saggi Mizrahi has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 4:
(1 comment)
....................................................
File vdsm/BindingXMLRPC.py
Line 476: domain = API.StorageDomain(sdUUID)
Line 477: return domain.format(autoDetach)
Line 478:
Line 479: def domainGetFileStats(self, sdUUID, pattern='*',
Line 480: caseSensitive=True, options=None):
I agree, even though normally this would be the right thing to do due to backward
compatibility since you are already changing the name of the verb you are throwing BC out
of the door anyway.
Line 481: domain = API.StorageDomain(sdUUID)
Line 482: return domain.getFileStats(pattern, caseSensitive)
Line 483:
Line 484: def domainGetImages(self, sdUUID, options=None):
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
6:41 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 3: Code-Review-1 Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5093/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4289/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5167/ : FAILURE
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:43 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
Sergey Gotliv has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 2:
I aware of the jenkins failure and will fix it later. I am just checking direction.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:40 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
Sergey Gotliv has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 2:
After all I decided to define 'caseSensitive' option explicitly instead of using
it as part of the options.
I checked all available options to use with regex in re.py and didn't find something
interesting that we may want to use in the future. I also didn't want to deal with the
question of how keep these options so Engine and VDSM will use the same constants like
IGNORECASE...
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:36 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 2: Code-Review-1 Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5092/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4288/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5166/ : FAILURE
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:39 a.m.
Nir Soffer has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
Sergey, Dan and me discussed the wanted api, and agreed that:
- Change the name to getFileStats.
- Use the options argument for the caseSensitive option, so we can add more options later.
Sergey will check how much work is needed for this, as options are never sent from the
engine to vdsm.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
6:37 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
Nir, how about passing the new "caseSensitive" flag as one of the
currently-unused options?
"options" is there for extendability, just like this one.
As for the name change - I like it a lot.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
5:05 a.m.
Sergey Gotliv has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
(2 comments)
All I need is return all available information about iso and vfd files from VDSM to Engine
in order to fix a bug.
My another patch that fixing that issue:
http://gerrit.ovirt.org/#/c/19544/
is approved by not merged - I think it deserves a new status :-).
I have no problem to change the name and signature of this API as Nir wishes, but only if
all (and mostly maintainer) will decide that this is the right direction.
....................................................
Commit Message
Line 3: AuthorDate: 2013-10-23 23:36:03 +0300
Line 4: Commit: Sergey Gotliv <sgotliv(a)redhat.com>
Line 5: CommitDate: 2013-10-23 23:57:55 +0300
Line 6:
Line 7: getFileList() filters files in case incensitive manner
Done
Line 8:
Line 9: Currently Engine is using getIsoList() and getFloppyList() APIs to
Line 10: retrieve the list of iso and floppy files respectively. Both these APIs
Line 11: are working in case incensitive manner but don't provide file
Line 10: retrieve the list of iso and floppy files respectively. Both these APIs
Line 11: are working in case incensitive manner but don't provide file
Line 12: statistics. Changing getFileList(), which is already providing full
Line 13: information about files, to be case incensitive will make it adoption
Line 14: easier.
Done
Line 15:
Line 16: Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
1:50 a.m.
Nir Soffer has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
(2 comments)
Hopefully it will be easy to get this minimal change merged, but in this case we should
consider a bigger change.
getFileList() was never used; this is our only chance to fix it:
- Fix the name; it should be getFileStats. Using List in an API is a poor choice.
- Add a caseSensitive flag
- Remove the unused options parameter
So a better change would be to depreciate getFileList and add a proper one instead.
....................................................
Commit Message
Line 3: AuthorDate: 2013-10-23 23:36:03 +0300
Line 4: Commit: Sergey Gotliv <sgotliv(a)redhat.com>
Line 5: CommitDate: 2013-10-23 23:57:55 +0300
Line 6:
Line 7: getFileList() filters files in case incensitive manner
Not clear if this is a description of the previous state or the new one. How about:
Use case insensitive search in getFileList()
Line 8:
Line 9: Currently Engine is using getIsoList() and getFloppyList() APIs to
Line 10: retrieve the list of iso and floppy files respectively. Both these APIs
Line 11: are working in case incensitive manner but don't provide file
Line 10: retrieve the list of iso and floppy files respectively. Both these APIs
Line 11: are working in case incensitive manner but don't provide file
Line 12: statistics. Changing getFileList(), which is already providing full
Line 13: information about files, to be case incensitive will make it adoption
Line 14: easier.
The last sentence could be more clear as:
getFileList() returns file statistics but use case sensitive search. Using case
insensitive search allow using getFileList() instead of getIsoList() and getFloppyList().
And dont forget to mention that:
getFileList() was never used in the engine, so this change should be safe.
Line 15:
Line 16: Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
7:19 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
Sergey Gotliv has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 4:
PEP8
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3:22 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
Theoretically speaking, there might exist a hermit that uses the getFileList API in its
current implementation. I do not consider this as a real risk, but I would not nack
keeping getFileList as an undocumented alias to getFileStats.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
1:04 p.m.
Sergey Gotliv has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
Dan,
getFileList() is exposed in vdsClient. Does it mean that customer potentially can use it?
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:02 a.m.
Nir Soffer has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
About the unused options parameter - if this is the convention in other apis, keep it.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
6:53 a.m.
Sergey Gotliv has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
Dan, please look at the Api.py.
None of the existing apis doesn't really accept options. Do you want this one to be
the first?
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
6:53 a.m.
Nir Soffer has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
I did not check yet other vdsm apis; if options already used like this, lets be consistent
and use options.caseSensitive. If not, explicit parameter is more clear.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:21 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5094/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4290/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5168/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:58 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
Saggi Mizrahi has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 4:
Removing myself as a reviewer, ping me if the interface file changes
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:13 p.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Nir Soffer has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 5:
Are you sure patch 5 is correct? I don't see the changes in gerrit.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
7:06 p.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Federico Simoncelli has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 6: Code-Review+1
I'm not thrilled by the caseSensitive argument. I'll think about it. The rest
looks good.
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:40 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: getFileList() filters files in case incensitive manner
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5043/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4239/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5117/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:56 p.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
Nir Soffer has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 4:
(12 comments)
Looks good but needs some cleanups.
....................................................
Commit Message
Line 3: AuthorDate: 2013-10-23 23:36:03 +0300
Line 4: Commit: Sergey Gotliv <sgotliv(a)redhat.com>
Line 5: CommitDate: 2013-10-26 15:18:52 +0300
Line 6:
Line 7: Refactor getFilelist()
This is not a refactoring - refactoring is improving the design of the code without
changing the behavior. This is change in the behavior.
I would describe it as:
Replace unused getFileList() with more flexible getFileStats()
Now it is very clear what you are doing here. and the rest of the commit message can
explain why getFileList needed to be replaced, note that this is a safe change, and
describe how getFileStats is more flexible.
Line 8:
Line 9: 1. Rename to getFileStats()
Line 10:
Line 11: New name getFileStats() better describes the essence of that API. This
Line 10:
Line 11: New name getFileStats() better describes the essence of that API. This
Line 12: is safe to rename it now because this API is not in use.
Line 13:
Line 14: 2. Allow to perform case incesitive search
The correct term is case-sensitive or case-insensitive.
See http://en.wikipedia.org/wiki/Case_sensitivity
Maybe:
2. Add caseSensitive option
Line 15:
Line 16: Currently Engine is using getIsoList() and getFloppyList() APIs to
Line 17: retrieve the list of iso and floppy files respectively. Both these APIs
Line 18: are working in case incensitive manner but don't provide file
Line 15:
Line 16: Currently Engine is using getIsoList() and getFloppyList() APIs to
Line 17: retrieve the list of iso and floppy files respectively. Both these APIs
Line 18: are working in case incensitive manner but don't provide file
Line 19: statistics.
Replace "case incensitive" with "case insensitive".
Line 20: getFileList() is currently not used. It returns file statistics but
Line 21: uses case sensitive search. Using case insensitive search
Line 22: allow using getFileList() instead of getIsoList() and getFloppyList().
Line 23:
Line 18: are working in case incensitive manner but don't provide file
Line 19: statistics.
Line 20: getFileList() is currently not used. It returns file statistics but
Line 21: uses case sensitive search. Using case insensitive search
Line 22: allow using getFileList() instead of getIsoList() and getFloppyList().
This was correct in some patches ago; now it should be:
Adding caseSensitive option allows using getFileStats() instead of getIsoList() and
getFloppyList().
Line 23:
Line 24: 3. Rename return value and exception to reflect the change of the API
Line 25: name.
Line 26:
Line 21: uses case sensitive search. Using case insensitive search
Line 22: allow using getFileList() instead of getIsoList() and getFloppyList().
Line 23:
Line 24: 3. Rename return value and exception to reflect the change of the API
Line 25: name.
This is obvious.
Line 26:
Line 27: Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Line 28: Bug-Url: https://bugzilla.redhat.com/1005889
....................................................
File vdsm/BindingXMLRPC.py
Line 476: domain = API.StorageDomain(sdUUID)
Line 477: return domain.format(autoDetach)
Line 478:
Line 479: def domainGetFileStats(self, sdUUID, pattern='*',
Line 480: caseSensitive=True, options=None):
I think caseSensitive default should be False since this is more useful when doing glob
style searches, for example searching ISO files ("*.iso" or "*.ISO").
However we should also be consistent; if the underlying getFileList we call use
caseSensitive=True, we should use the same default.
Line 481: domain = API.StorageDomain(sdUUID)
Line 482: return domain.getFileStats(pattern, caseSensitive)
Line 483:
Line 484: def domainGetImages(self, sdUUID, options=None):
....................................................
File vdsm/storage/hsm.py
Line 2225: return self.taskMng.revertTask(taskID=taskID)
Line 2226:
Line 2227: @public
Line 2228: def getFileStats(self, sdUUID, pattern='*', caseSensitive=True,
Line 2229: options=None):
Match caseSensitive default with BindingXMLRPC.py
Line 2230: """
Line 2231: Returns statistics of all files in the domain filtered according to
Line 2232: pattern.
Line 2233:
Line 2234: :param sdUUID: The UUID of the storage domain you want to query.
Line 2235: :type sdUUID: UUID
Line 2236: :param pattern: The glob expression for filtering.
Line 2237: :type pattern: str
Line 2238: :param caseSensitive: Controls the case of the matching.
Not clear - check how similar apis describe this feature.
Line 2239: :type caseSensitive: bool
Line 2240: :options: ?
Line 2241:
Line 2242: :returns: a dict of all the volumes found.
Line 2238: :param caseSensitive: Controls the case of the matching.
Line 2239: :type caseSensitive: bool
Line 2240: :options: ?
Line 2241:
Line 2242: :returns: a dict of all the volumes found.
volumes?
Line 2243: :rtype: dict
Line 2244: """
Line 2245: vars.task.setDefaultException(se.GetFileStatsError(sdUUID))
Line 2246: vars.task.getSharedLock(STORAGE, sdUUID)
Line 2249: if not dom.isISO or dom.getStorageType() != sd.NFS_DOMAIN:
Line 2250: raise se.GetFileStatsError(sdUUID)
Line 2251:
Line 2252: filesDict = dom.getFileList(pattern=pattern,
Line 2253: caseSensitive=caseSensitive)
Replace filesDict with fileStats.
Line 2254: return {'fileStats': filesDict}
Line 2255:
Line 2256: @public
Line 2257: def getIsoList(self, spUUID, extension='iso', options=None):
....................................................
File vdsm/storage/storage_exception.py
Line 606:
Line 607:
Line 608: class GetFileStatsError(StorageException):
Line 609: code = 330
Line 610: message = "Cannot get files stats"
Replace files with file.
Line 611:
Line 612:
Line 613: #################################################
Line 614: # Domains Exceptions
....................................................
File vdsm_api/vdsmapi-schema.json
Line 3840: # @storagedomainID: The UUID of the Storage Domain
Line 3841: #
Line 3842: # @pattern: Filter results by this glob pattern
Line 3843: #
Line 3844: # @caseSensitive: Controls the case of the matching
Not clear - match getFileStats docstring.
Line 3845: #
Line 3846: # Returns:
Line 3847: # A dictionary of file information indexed by file name
Line 3848: #
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
11:49 a.m.
New subject: Change in vdsm[master]: Refactor getFilelist()
Sergey Gotliv has posted comments on this change.
Change subject: Refactor getFilelist()
......................................................................
Patch Set 4:
(12 comments)
....................................................
Commit Message
Line 3: AuthorDate: 2013-10-23 23:36:03 +0300
Line 4: Commit: Sergey Gotliv <sgotliv(a)redhat.com>
Line 5: CommitDate: 2013-10-26 15:18:52 +0300
Line 6:
Line 7: Refactor getFilelist()
Done
Line 8:
Line 9: 1. Rename to getFileStats()
Line 10:
Line 11: New name getFileStats() better describes the essence of that API. This
Line 10:
Line 11: New name getFileStats() better describes the essence of that API. This
Line 12: is safe to rename it now because this API is not in use.
Line 13:
Line 14: 2. Allow to perform case incesitive search
Done
Line 15:
Line 16: Currently Engine is using getIsoList() and getFloppyList() APIs to
Line 17: retrieve the list of iso and floppy files respectively. Both these APIs
Line 18: are working in case incensitive manner but don't provide file
Line 15:
Line 16: Currently Engine is using getIsoList() and getFloppyList() APIs to
Line 17: retrieve the list of iso and floppy files respectively. Both these APIs
Line 18: are working in case incensitive manner but don't provide file
Line 19: statistics.
Done
Line 20: getFileList() is currently not used. It returns file statistics but
Line 21: uses case sensitive search. Using case insensitive search
Line 22: allow using getFileList() instead of getIsoList() and getFloppyList().
Line 23:
Line 18: are working in case incensitive manner but don't provide file
Line 19: statistics.
Line 20: getFileList() is currently not used. It returns file statistics but
Line 21: uses case sensitive search. Using case insensitive search
Line 22: allow using getFileList() instead of getIsoList() and getFloppyList().
Done
Line 23:
Line 24: 3. Rename return value and exception to reflect the change of the API
Line 25: name.
Line 26:
Line 21: uses case sensitive search. Using case insensitive search
Line 22: allow using getFileList() instead of getIsoList() and getFloppyList().
Line 23:
Line 24: 3. Rename return value and exception to reflect the change of the API
Line 25: name.
Yes. CaseSensitive is obvious either. :-)
Line 26:
Line 27: Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Line 28: Bug-Url: https://bugzilla.redhat.com/1005889
....................................................
File vdsm/BindingXMLRPC.py
Line 476: domain = API.StorageDomain(sdUUID)
Line 477: return domain.format(autoDetach)
Line 478:
Line 479: def domainGetFileStats(self, sdUUID, pattern='*',
Line 480: caseSensitive=True, options=None):
Done
Line 481: domain = API.StorageDomain(sdUUID)
Line 482: return domain.getFileStats(pattern, caseSensitive)
Line 483:
Line 484: def domainGetImages(self, sdUUID, options=None):
....................................................
File vdsm/storage/hsm.py
Line 2225: return self.taskMng.revertTask(taskID=taskID)
Line 2226:
Line 2227: @public
Line 2228: def getFileStats(self, sdUUID, pattern='*', caseSensitive=True,
Line 2229: options=None):
There is a match. Now it true, I'll change it to false.
Line 2230: """
Line 2231: Returns statistics of all files in the domain filtered according to
Line 2232: pattern.
Line 2233:
Line 2234: :param sdUUID: The UUID of the storage domain you want to query.
Line 2235: :type sdUUID: UUID
Line 2236: :param pattern: The glob expression for filtering.
Line 2237: :type pattern: str
Line 2238: :param caseSensitive: Controls the case of the matching.
I think this is so obvious that doesn't need any explanation.
This is explanation of IGNORECASE flag in re.py
"I IGNORECASE Perform case-insensitive matching."
This is from javadoc:
public static final int CASE_INSENSITIVE
Enables case-insensitive matching.
Let's see, without these explanations above I would never guess what IGNORECASE
means.
I will align my explanation with those abobe, but if you'll provide something better,
I'll take it.
Line 2239: :type caseSensitive: bool
Line 2240: :options: ?
Line 2241:
Line 2242: :returns: a dict of all the volumes found.
Line 2238: :param caseSensitive: Controls the case of the matching.
Line 2239: :type caseSensitive: bool
Line 2240: :options: ?
Line 2241:
Line 2242: :returns: a dict of all the volumes found.
Done
Line 2243: :rtype: dict
Line 2244: """
Line 2245: vars.task.setDefaultException(se.GetFileStatsError(sdUUID))
Line 2246: vars.task.getSharedLock(STORAGE, sdUUID)
Line 2249: if not dom.isISO or dom.getStorageType() != sd.NFS_DOMAIN:
Line 2250: raise se.GetFileStatsError(sdUUID)
Line 2251:
Line 2252: filesDict = dom.getFileList(pattern=pattern,
Line 2253: caseSensitive=caseSensitive)
Done
Line 2254: return {'fileStats': filesDict}
Line 2255:
Line 2256: @public
Line 2257: def getIsoList(self, spUUID, extension='iso', options=None):
....................................................
File vdsm/storage/storage_exception.py
Line 606:
Line 607:
Line 608: class GetFileStatsError(StorageException):
Line 609: code = 330
Line 610: message = "Cannot get files stats"
Done
Line 611:
Line 612:
Line 613: #################################################
Line 614: # Domains Exceptions
....................................................
File vdsm_api/vdsmapi-schema.json
Line 3840: # @storagedomainID: The UUID of the Storage Domain
Line 3841: #
Line 3842: # @pattern: Filter results by this glob pattern
Line 3843: #
Line 3844: # @caseSensitive: Controls the case of the matching
Its the same explanation as in the docstring.
Line 3845: #
Line 3846: # Returns:
Line 3847: # A dictionary of file information indexed by file name
Line 3848: #
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
3:08 a.m.
New subject: Change in vdsm[master]: Replace unused getFileList() with more flexible getFileStats()
Ayal Baron has posted comments on this change.
Change subject: Replace unused getFileList() with more flexible getFileStats()
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/20476
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I550827c7b4c7e11fe09e41745fcc9d91249c6c23
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3827
days inactive
3837
days old
vdsm-patches@lists.fedorahosted.org
32 comments
7 participants
participants (7)
-
abaron＠redhat.com -
Dan Kenigsberg -
Federico Simoncelli -
Jenkins CI RO -
Nir Soffer -
sgotliv＠redhat.com -
smizrahi＠redhat.com