PR#1186: new XMLRPC method: listFiles - koji

koji

#1186 new XMLRPC method: listFiles

Closed 4 years ago by mikem. Opened 5 years ago by vmaljulin.

vmaljulin/koji FACTORY-3473 into master

new XMLRPC method: listFiles

Valerij Maljulin • 5 years ago

5faf619

hub/kojihub.py

file modified

+39 -1

		`@@ -9027,6 +9027,45 @@`
		`context.session.assertPerm('admin')`
		`return make_task(args, *opts)`

		`+ def listFiles(self, path_mask, volume=None):`
		`+ '''`
		`+ retrive a file list from koji`
		`+ :param path_mask: relative path mask (* to replace part of the path)`
		`+ could be used like "mbs/-19/.json"`
		`+ :return: dict containing file list, keys are file/dir name`
		`+ '''`
		`+ path_mask = os.path.normpath(path_mask)`
		`+ if not path_mask or path_mask.startswith('..'):`
		`+ raise koji.GenericError("Invalid path mask: %s" % path_mask)`
		`+ if volume is not None:`
		`+ # make sure the volume is valid`
		`+ lookup_name('volume', volume, strict=True)`
		`+ parts = path_mask.split('/')`
		`+ list_root_path = koji.pathinfo.work(volume=volume)`
		`+ return self._list_dir(list_root_path, parts)`
		`+`
		`+ @classmethod`
		`+ def _list_dir(cls, list_root_path, path_parts, level=0):`
		`+ ret_val = {'files': {}, 'subdirs': {}}`
		`+ if level < len(path_parts):`
		`+ template = path_parts[level]`
		`+ else:`
		`+ template = '*'`
		`+ template = template.replace('.', r'\.')`
		`+ template = template.replace('', r'.')`
		`+ template_re = re.compile(r'^' + template + r'$')`
		`+ for dir_entry in os.listdir(list_root_path):`
		`+ if dir_entry.startswith('.'):`
		`+ continue`
		`+ if not template_re.match(dir_entry):`
		`+ continue`
		`+ full_path = os.path.join(list_root_path, dir_entry)`
		`+ if os.path.isdir(full_path):`
		`+ ret_val['subdirs'][dir_entry] = cls._list_dir(full_path, path_parts, level + 1)`
		`+ else:`
		`+ ret_val['files'][dir_entry] = {'size': os.path.getsize(full_path)}`
		`+ return ret_val`
		`+`
		`def uploadFile(self, path, name, size, md5sum, offset, data, volume=None):`
		`#path: the relative path to upload to`
		`#name: the name of the file`
		`@@ -9167,7 +9206,6 @@`
		`# this will also free our lock`
		`os.close(fd)`

		`-`
		`def downloadTaskOutput(self, taskID, fileName, offset=0, size=-1, volume=None):`
		`"""Download the file with the given name, generated by the task with the`
		`given ID."""`

tests/test_hub/test_list_files.py

file added

+100

		`@@ -0,0 +1,100 @@`
		`+ from __future__ import absolute_import`
		`+ import mock`
		`+ try:`
		`+ import unittest2 as unittest`
		`+ except ImportError:`
		`+ import unittest`
		`+`
		`+ import kojihub`
		`+ import os`
		`+ import random`
		`+ from tempfile import gettempdir`
		`+`
		`+`
		`+ def get_temp_dir_root():`
		`+ return os.path.join(gettempdir(), 'koji_tests')`
		`+`
		`+`
		`+ def get_tmp_dir_path(folder_starts_with):`
		`+ return os.path.join(get_temp_dir_root(), ('{0}{1}'.format(folder_starts_with, random.randint(1, 999999999999))))`
		`+`
		`+`
		`+ class TestListFiles(unittest.TestCase):`
		`+ def setUp(self):`
		`+ self.hub = kojihub.RootExports()`
		`+ self.standard_processor_kwargs = dict(`
		`+ tables=mock.ANY,`
		`+ columns=mock.ANY,`
		`+ values=mock.ANY,`
		`+ joins=mock.ANY,`
		`+ clauses=mock.ANY,`
		`+ opts=mock.ANY,`
		`+ aliases=mock.ANY,`
		`+ )`
		`+`
		`+ @mock.patch('koji.pathinfo.work')`
		`+ def test_root_exports_listFiles(self, koji_pathinfo_work):`
		`+ temp_path = get_tmp_dir_path('TestTask')`
		`+ koji_pathinfo_work.return_value = temp_path`
		`+ structure_to_create = {`
		`+ 'subdirs': {`
		`+ 'dir1': {`
		`+ 'subdirs': {`
		`+ 'dir2': {`
		`+ 'subdirs': {},`
		`+ 'files': {`
		`+ 'file3': {'size': 30},`
		`+ 'file4': {'size': 7}`
		`+ }`
		`+ },`
		`+ 'dir3': {`
		`+ 'subdirs': {},`
		`+ 'files': {`
		`+ 'file5': {'size': 17},`
		`+ 'file6': {'size': 23}`
		`+ }`
		`+ }`
		`+ },`
		`+ 'files': {`
		`+ 'file1': {'size': 20},`
		`+ 'file2': {'size': 10}`
		`+ }`
		`+ }`
		`+ },`
		`+ 'files': {}`
		`+ }`
		`+ test_structure_2 = {`
		`+ 'subdirs': {`
		`+ 'dir1': {`
		`+ 'subdirs': {`
		`+ 'dir2': {`
		`+ 'subdirs': {},`
		`+ 'files': {`
		`+ 'file3': {'size': 30},`
		`+ 'file4': {'size': 7}`
		`+ }`
		`+ }`
		`+ },`
		`+ 'files': {`
		`+ 'file2': {'size': 10}`
		`+ }`
		`+ }`
		`+ },`
		`+ 'files': {}`
		`+ }`
		`+ self._create_dir_structure(temp_path, structure_to_create)`
		`+`
		`+ self.assertDictEqual(self.hub.listFiles('dir1/*'), structure_to_create)`
		`+ self.assertDictEqual(self.hub.listFiles('dir1/*2'), test_structure_2)`
		`+`
		`+ def _create_dir_structure(self, current_dir, dir_struct):`
		`+ os.makedirs(current_dir)`
		`+ for k in dir_struct.get('subdirs', {}).keys():`
		`+ sub_path = os.path.join(current_dir, k)`
		`+ self._create_dir_structure(sub_path, dir_struct['subdirs'][k])`
		`+ for k in dir_struct.get('files', {}).keys():`
		`+ sub_path = os.path.join(current_dir, k)`
		`+ with open(sub_path, 'w+b') as new_file:`
		`+ with open('/dev/urandom') as urand_file:`
		`+ new_file.write(urand_file.read(dir_struct['files'][k]['size']))`
		`+`

vmaljulin commented 5 years ago

used for MBS (fm-orchestrator)

Signed-off-by: Valerij Maljulin vmaljuli@redhat.com

ktdreyer commented on line 8 of hub/kojihub.py 5 years ago

Would you please give an example path_mask value here? And add a docstring for the "volume" parameter?

rebased onto 5faf619

5 years ago

vmaljulin commented 5 years ago

Would you please give an example path_mask value here? And add a docstring for the "volume" parameter?

I'm not really sure about this volume parameter. I'd probably never use it, but it's used by the koji.pathinfo.work function. The first part of your note: done.

lsedlar commented 5 years ago

Out of curiosity, can you give an example where this would be useful?

vmaljulin commented 5 years ago

Out of curiosity, can you give an example where this would be useful?

It could be used for reusing a previously uploaded data, so we don't need to upload them twice.

mikem commented 5 years ago

Please do not add non-exported methods inside of RootExports. Yes, there may be a few very old ones in there, but we do not need to compound any such errors.

mikem commented 5 years ago

I am opposed to providing this sort of global files api. Koji is not simply a file server. This call provides unstructured views into the file structure. This is too much like having apps browse the filesystem directly (something we discourage).

Can we please start with an issue detailing your current needs that led to this work?

vmaljulin commented 5 years ago

Please do not add non-exported methods inside of RootExports. Yes, there may be a few very old ones in there, but we do not need to compound any such errors.

where should I place it then? Maybe it will be enough to rename it so it starts from "__" (double underscore)?

mikem commented 5 years ago