From 08c91e0dd7701419265eff0363b258a0d8bd27ab Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Thu, 15 Oct 2020 15:14:46 -0400 Subject: [PATCH 1/7] Revised code so an irods session is created and maintained. We are updating python-irodsclient to deal with stale/dropped connections. --- lib/galaxy/objectstore/irods.py | 385 ++++++++++++++++---------------- 1 file changed, 194 insertions(+), 191 deletions(-) diff --git a/lib/galaxy/objectstore/irods.py b/lib/galaxy/objectstore/irods.py index e126912c3a4..25faac71888 100644 --- a/lib/galaxy/objectstore/irods.py +++ b/lib/galaxy/objectstore/irods.py @@ -4,7 +4,6 @@ Object Store plugin for the Integrated Rule-Oriented Data Store (iRODS) import logging import os import shutil -from contextlib import contextmanager from datetime import datetime from functools import partial try: @@ -111,31 +110,6 @@ def parse_config_xml(config_xml): raise -def acquire_session(host='localhost', port='1247', user='rods', password='rods', zone='tempZone', timeout='30'): - session = iRODSSession(host=host, port=port, user=user, password=password, zone=zone) - # Set connection timeout - session.connection_timeout = timeout - return session - - -def release_session(session): - # This call will cleanup all the connections in the connection pool - # OSError sometimes happens on GitHub Actions, after the test has successfully completed. Ignore it if it happens. - try: - session.cleanup() - except OSError: - pass - - -@contextmanager -def managed_session(host='localhost', port='1247', user='rods', password='rods', zone='tempZone', timeout='30'): - session = acquire_session(host=host, port=port, user=user, password=password, zone=zone, timeout=timeout) - try: - yield session - finally: - release_session(session) - - class CloudConfigMixin: def _config_to_dict(self): @@ -235,8 +209,42 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): raise Exception(IRODS_IMPORT_MESSAGE) self.home = "/" + self.zone + "/home/" + self.username + self._initialize() log.debug("irods __init__ %s", reload_timer) + + def shutdown(self): + # This call will cleanup all the connections in the connection pool + # OSError sometimes happens on GitHub Actions, after the test has successfully completed. Ignore it if it happens. + try: + self.session.cleanup() + except OSError: + pass + + def _initialize(self): + if irods is None: + raise Exception(IRODS_IMPORT_MESSAGE) + + self.session = self._configure_connection(host=self.host, port=self.port, user=self.username, password=self.password, zone=self.zone, timeout=self.timeout, poolsize=self.poolsize) + + def _configure_connection(self, host='localhost', port='1247', user='rods', password='rods', zone='tempZone', timeout=30, poolsize=1): + session = iRODSSession(host=host, port=port, user=user, password=password, zone=zone) + # Set connection timeout + session.connection_timeout = timeout + # Throws NetworkException if connection fails + try: + # We will create as many conections as poolsize. + # After creating the connection, release it + # so it goes into the idle connection queue + for idx in range(poolsize): + conn = session.pool.get_connection() + session.pool.release_connection(conn) + except NetworkException as e: + log.error('Could not create iRODS session: ' + str(e)) + raise + return session + + @classmethod def parse_xml(cls, config_xml): return parse_config_xml(config_xml) @@ -295,43 +303,41 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): # rel_path is file or folder? def _get_size_in_irods(self, rel_path): - with managed_session(host=self.host, port=self.port, user=self.username, password=self.password, zone=self.zone, timeout=self.timeout) as session: - p = Path(rel_path) - data_object_name = p.stem + p.suffix - subcollection_name = p.parent + p = Path(rel_path) + data_object_name = p.stem + p.suffix + subcollection_name = p.parent - collection_path = self.home + "/" + str(subcollection_name) - data_object_path = collection_path + "/" + str(data_object_name) + collection_path = self.home + "/" + str(subcollection_name) + data_object_path = collection_path + "/" + str(data_object_name) - try: - data_obj = session.data_objects.get(data_object_path) - return data_obj.__sizeof__() - except (DataObjectDoesNotExist, CollectionDoesNotExist): - log.warn("Collection or data object (%s) does not exist", data_object_path) - return -1 - except NetworkException as e: - log.exception(e) - return -1 + try: + data_obj = self.session.data_objects.get(data_object_path) + return data_obj.__sizeof__() + except (DataObjectDoesNotExist, CollectionDoesNotExist): + log.warn("Collection or data object (%s) does not exist", data_object_path) + return -1 + except NetworkException as e: + log.exception(e) + return -1 # rel_path is file or folder? def _data_object_exists(self, rel_path): - with managed_session(host=self.host, port=self.port, user=self.username, password=self.password, zone=self.zone, timeout=self.timeout) as session: - p = Path(rel_path) - data_object_name = p.stem + p.suffix - subcollection_name = p.parent + p = Path(rel_path) + data_object_name = p.stem + p.suffix + subcollection_name = p.parent - collection_path = self.home + "/" + str(subcollection_name) - data_object_path = collection_path + "/" + str(data_object_name) + collection_path = self.home + "/" + str(subcollection_name) + data_object_path = collection_path + "/" + str(data_object_name) - try: - session.data_objects.get(data_object_path) - return True - except (DataObjectDoesNotExist, CollectionDoesNotExist): - log.debug("Collection or data object (%s) does not exist", data_object_path) - return False - except NetworkException as e: - log.exception(e) - return False + try: + self.session.data_objects.get(data_object_path) + return True + except (DataObjectDoesNotExist, CollectionDoesNotExist): + log.debug("Collection or data object (%s) does not exist", data_object_path) + return False + except NetworkException as e: + log.exception(e) + return False def _in_cache(self, rel_path): """ Check if the given dataset is in the local cache and return True if so. """ @@ -349,37 +355,36 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): return file_ok def _download(self, rel_path): - with managed_session(host=self.host, port=self.port, user=self.username, password=self.password, zone=self.zone, timeout=self.timeout) as session: - log.debug("Pulling data object '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) + log.debug("Pulling data object '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) - p = Path(rel_path) - data_object_name = p.stem + p.suffix - subcollection_name = p.parent + p = Path(rel_path) + data_object_name = p.stem + p.suffix + subcollection_name = p.parent - collection_path = self.home + "/" + str(subcollection_name) - data_object_path = collection_path + "/" + str(data_object_name) - data_obj = None + collection_path = self.home + "/" + str(subcollection_name) + data_object_path = collection_path + "/" + str(data_object_name) + data_obj = None - try: - data_obj = session.data_objects.get(data_object_path) - except (DataObjectDoesNotExist, CollectionDoesNotExist): - log.warn("Collection or data object (%s) does not exist", data_object_path) - return False - except NetworkException as e: - log.exception(e) - return False + try: + data_obj = self.session.data_objects.get(data_object_path) + except (DataObjectDoesNotExist, CollectionDoesNotExist): + log.warn("Collection or data object (%s) does not exist", data_object_path) + return False + except NetworkException as e: + log.exception(e) + return False - if self.cache_size > 0 and data_obj.__sizeof__() > self.cache_size: - log.critical("File %s is larger (%s) than the cache size (%s). Cannot download.", - rel_path, data_obj.__sizeof__(), self.cache_size) - return False + if self.cache_size > 0 and data_obj.__sizeof__() > self.cache_size: + log.critical("File %s is larger (%s) than the cache size (%s). Cannot download.", + rel_path, data_obj.__sizeof__(), self.cache_size) + return False - log.debug("Pulled data object '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) + log.debug("Pulled data object '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) - with data_obj.open('r') as data_obj_fp, open(self._get_cache_path(rel_path), "wb") as cache_fp: - for chunk in iter(partial(data_obj_fp.read, CHUNK_SIZE), b''): - cache_fp.write(chunk) - return True + with data_obj.open('r') as data_obj_fp, open(self._get_cache_path(rel_path), "wb") as cache_fp: + for chunk in iter(partial(data_obj_fp.read, CHUNK_SIZE), b''): + cache_fp.write(chunk) + return True def _push_to_irods(self, rel_path, source_file=None, from_string=None): """ @@ -390,54 +395,53 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): still using ``rel_path`` for collection and object store names. If ``from_string`` is provided, set contents of the file to the value of the string. """ - with managed_session(host=self.host, port=self.port, user=self.username, password=self.password, zone=self.zone, timeout=self.timeout) as session: - p = Path(rel_path) - data_object_name = p.stem + p.suffix - subcollection_name = p.parent + p = Path(rel_path) + data_object_name = p.stem + p.suffix + subcollection_name = p.parent - source_file = source_file if source_file else self._get_cache_path(rel_path) - options = {kw.FORCE_FLAG_KW: ''} + source_file = source_file if source_file else self._get_cache_path(rel_path) + options = {kw.FORCE_FLAG_KW: ''} - if os.path.exists(source_file): - # Check if the data object exists in iRODS - collection_path = self.home + "/" + str(subcollection_name) - data_object_path = collection_path + "/" + str(data_object_name) - exists = False - try: - exists = session.data_objects.exists(data_object_path) + if os.path.exists(source_file): + # Check if the data object exists in iRODS + collection_path = self.home + "/" + str(subcollection_name) + data_object_path = collection_path + "/" + str(data_object_name) + exists = False + try: + exists = self.session.data_objects.exists(data_object_path) - if os.path.getsize(source_file) == 0 and exists: - log.debug("Wanted to push file '%s' to iRODS collection '%s' but its size is 0; skipping.", source_file, rel_path) - return True - - if from_string: - data_obj = session.data_objects.create(data_object_path, self.resource, **options) - with data_obj.open('w') as data_obj_fp: - data_obj_fp.write(from_string) - log.debug("Pushed data from string '%s' to collection '%s'", from_string, data_object_path) - else: - start_time = datetime.now() - log.debug("Pushing cache file '%s' of size %s bytes to collection '%s'", source_file, os.path.getsize(source_file), rel_path) - - # Create sub-collection first - session.collections.create(collection_path, recurse=True) - data_obj = session.data_objects.create(data_object_path, self.resource, **options) - - # Write to file in subcollection created above - with open(source_file, 'rb') as content_file, data_obj.open('w') as data_obj_fp: - for chunk in iter(partial(content_file.read, CHUNK_SIZE), b''): - data_obj_fp.write(chunk) - - end_time = datetime.now() - log.debug("Pushed cache file '%s' to collection '%s' (%s bytes transfered in %s sec)", - source_file, rel_path, os.path.getsize(source_file), end_time - start_time) + if os.path.getsize(source_file) == 0 and exists: + log.debug("Wanted to push file '%s' to iRODS collection '%s' but its size is 0; skipping.", source_file, rel_path) return True - except NetworkException as e: - log.exception(e) - return False - log.error("Tried updating key '%s' from source file '%s', but source file does not exist.", rel_path, source_file) - return False + if from_string: + data_obj = self.session.data_objects.create(data_object_path, self.resource, **options) + with data_obj.open('w') as data_obj_fp: + data_obj_fp.write(from_string) + log.debug("Pushed data from string '%s' to collection '%s'", from_string, data_object_path) + else: + start_time = datetime.now() + log.debug("Pushing cache file '%s' of size %s bytes to collection '%s'", source_file, os.path.getsize(source_file), rel_path) + + # Create sub-collection first + self.session.collections.create(collection_path, recurse=True) + data_obj = self.session.data_objects.create(data_object_path, self.resource, **options) + + # Write to file in subcollection created above + with open(source_file, 'rb') as content_file, data_obj.open('w') as data_obj_fp: + for chunk in iter(partial(content_file.read, CHUNK_SIZE), b''): + data_obj_fp.write(chunk) + + end_time = datetime.now() + log.debug("Pushed cache file '%s' to collection '%s' (%s bytes transfered in %s sec)", + source_file, rel_path, os.path.getsize(source_file), end_time - start_time) + return True + except NetworkException as e: + log.exception(e) + return False + + log.error("Tried updating key '%s' from source file '%s', but source file does not exist.", rel_path, source_file) + return False def file_ready(self, obj, **kwargs): """ @@ -518,76 +522,75 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): return 0 def _delete(self, obj, entire_dir=False, **kwargs): - with managed_session(host=self.host, port=self.port, user=self.username, password=self.password, zone=self.zone, timeout=self.timeout) as session: - rel_path = self._construct_path(obj, **kwargs) - extra_dir = kwargs.get('extra_dir', None) - base_dir = kwargs.get('base_dir', None) - dir_only = kwargs.get('dir_only', False) - obj_dir = kwargs.get('obj_dir', False) + rel_path = self._construct_path(obj, **kwargs) + extra_dir = kwargs.get('extra_dir', None) + base_dir = kwargs.get('base_dir', None) + dir_only = kwargs.get('dir_only', False) + obj_dir = kwargs.get('obj_dir', False) - try: - # Remove temparory data in JOB_WORK directory - if base_dir and dir_only and obj_dir: - shutil.rmtree(os.path.abspath(rel_path)) + try: + # Remove temparory data in JOB_WORK directory + if base_dir and dir_only and obj_dir: + shutil.rmtree(os.path.abspath(rel_path)) + return True + + # For the case of extra_files, because we don't have a reference to + # individual files we need to remove the entire directory structure + # with all the files in it. This is easy for the local file system, + # but requires iterating through each individual key in irods and deleing it. + if entire_dir and extra_dir: + shutil.rmtree(self._get_cache_path(rel_path)) + + col_path = self.home + "/" + str(rel_path) + col = None + try: + col = self.session.collections.get(col_path) + except CollectionDoesNotExist: + log.warn("Collection (%s) does not exist!", col_path) + return False + except NetworkException as e: + log.exception(e) + return False + + cols = col.walk() + # Traverse the tree only one level deep + for _ in range(2): + # get next result + _, _, data_objects = next(cols) + + # Delete data objects + for data_object in data_objects: + data_object.unlink(force=True) + + return True + + else: + # Delete from cache first + os.unlink(self._get_cache_path(rel_path)) + # Delete from irods as well + p = Path(rel_path) + data_object_name = p.stem + p.suffix + subcollection_name = p.parent + + collection_path = self.home + "/" + str(subcollection_name) + data_object_path = collection_path + "/" + str(data_object_name) + + try: + data_obj = self.session.data_objects.get(data_object_path) + # remove object + data_obj.unlink(force=True) return True - - # For the case of extra_files, because we don't have a reference to - # individual files we need to remove the entire directory structure - # with all the files in it. This is easy for the local file system, - # but requires iterating through each individual key in irods and deleing it. - if entire_dir and extra_dir: - shutil.rmtree(self._get_cache_path(rel_path)) - - col_path = self.home + "/" + str(rel_path) - col = None - try: - col = session.collections.get(col_path) - except CollectionDoesNotExist: - log.warn("Collection (%s) does not exist!", col_path) - return False - except NetworkException as e: - log.exception(e) - return False - - cols = col.walk() - # Traverse the tree only one level deep - for _ in range(2): - # get next result - _, _, data_objects = next(cols) - - # Delete data objects - for data_object in data_objects: - data_object.unlink(force=True) - + except (DataObjectDoesNotExist, CollectionDoesNotExist): + log.info("Collection or data object (%s) does not exist", data_object_path) return True - - else: - # Delete from cache first - os.unlink(self._get_cache_path(rel_path)) - # Delete from irods as well - p = Path(rel_path) - data_object_name = p.stem + p.suffix - subcollection_name = p.parent - - collection_path = self.home + "/" + str(subcollection_name) - data_object_path = collection_path + "/" + str(data_object_name) - - try: - data_obj = session.data_objects.get(data_object_path) - # remove object - data_obj.unlink(force=True) - return True - except (DataObjectDoesNotExist, CollectionDoesNotExist): - log.info("Collection or data object (%s) does not exist", data_object_path) - return True - except NetworkException as e: - log.exception(e) - return False - except OSError: - log.exception('%s delete error', self._get_filename(obj, **kwargs)) - except NetworkException as e: - log.exception(e) - return False + except NetworkException as e: + log.exception(e) + return False + except OSError: + log.exception('%s delete error', self._get_filename(obj, **kwargs)) + except NetworkException as e: + log.exception(e) + return False def _get_data(self, obj, start=0, count=-1, **kwargs): rel_path = self._construct_path(obj, **kwargs) From 606da705b581d55e3772191a8a8d6c71504b4df6 Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Fri, 16 Oct 2020 10:30:40 -0400 Subject: [PATCH 2/7] Pointing to my Git repo for python-irodsclient, so we can test irods connection refresh on Test. --- lib/galaxy/dependencies/conditional-requirements.txt | 3 ++- .../dependencies/pipfiles/default/pinned-dev-requirements.txt | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/dependencies/conditional-requirements.txt b/lib/galaxy/dependencies/conditional-requirements.txt index d59b712c2d8..fdf9822d862 100644 --- a/lib/galaxy/dependencies/conditional-requirements.txt +++ b/lib/galaxy/dependencies/conditional-requirements.txt @@ -8,7 +8,8 @@ drmaa statsd docker azure-storage==0.32.0 -python-irodsclient==0.8.3 +# python-irodsclient==0.8.3 +# git+https://github.com/kxk302/python-irodsclient.git python-ldap==3.2.0 python-pam galaxycloudrunner diff --git a/lib/galaxy/dependencies/pipfiles/default/pinned-dev-requirements.txt b/lib/galaxy/dependencies/pipfiles/default/pinned-dev-requirements.txt index b66360f78b3..ab2977c1e05 100644 --- a/lib/galaxy/dependencies/pipfiles/default/pinned-dev-requirements.txt +++ b/lib/galaxy/dependencies/pipfiles/default/pinned-dev-requirements.txt @@ -41,7 +41,8 @@ pytest-mock==3.1.1 pytest-postgresql==2.3.0 pytest-pythonpath==0.7.3 pytest==5.4.3 -python-irodsclient==0.8.3 +# python-irodsclient==0.8.3 +git+https://github.com/kxk302/python-irodsclient.git pytz==2020.1 recommonmark==0.6.0 requests==2.24.0 From ff85cf9ec044e7b88727179e1c10815ddc8873af Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Fri, 16 Oct 2020 11:05:42 -0400 Subject: [PATCH 3/7] Fixed lint issues. --- lib/galaxy/objectstore/irods.py | 106 ++++++++++++++++---------------- 1 file changed, 52 insertions(+), 54 deletions(-) diff --git a/lib/galaxy/objectstore/irods.py b/lib/galaxy/objectstore/irods.py index 65388cabc97..7c8938835e8 100644 --- a/lib/galaxy/objectstore/irods.py +++ b/lib/galaxy/objectstore/irods.py @@ -212,7 +212,6 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): self._initialize() log.debug("irods __init__ %s", reload_timer) - def shutdown(self): # This call will cleanup all the connections in the connection pool # OSError sometimes happens on GitHub Actions, after the test has successfully completed. Ignore it if it happens. @@ -244,7 +243,6 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): raise return session - @classmethod def parse_xml(cls, config_xml): return parse_config_xml(config_xml) @@ -303,41 +301,41 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): # rel_path is file or folder? def _get_size_in_irods(self, rel_path): - p = Path(rel_path) - data_object_name = p.stem + p.suffix - subcollection_name = p.parent + p = Path(rel_path) + data_object_name = p.stem + p.suffix + subcollection_name = p.parent - collection_path = self.home + "/" + str(subcollection_name) - data_object_path = collection_path + "/" + str(data_object_name) + collection_path = self.home + "/" + str(subcollection_name) + data_object_path = collection_path + "/" + str(data_object_name) - try: - data_obj = self.session.data_objects.get(data_object_path) - return data_obj.__sizeof__() - except (DataObjectDoesNotExist, CollectionDoesNotExist): - log.warn("Collection or data object (%s) does not exist", data_object_path) - return -1 - except NetworkException as e: - log.exception(e) - return -1 + try: + data_obj = self.session.data_objects.get(data_object_path) + return data_obj.__sizeof__() + except (DataObjectDoesNotExist, CollectionDoesNotExist): + log.warn("Collection or data object (%s) does not exist", data_object_path) + return -1 + except NetworkException as e: + log.exception(e) + return -1 # rel_path is file or folder? def _data_object_exists(self, rel_path): - p = Path(rel_path) - data_object_name = p.stem + p.suffix - subcollection_name = p.parent + p = Path(rel_path) + data_object_name = p.stem + p.suffix + subcollection_name = p.parent - collection_path = self.home + "/" + str(subcollection_name) - data_object_path = collection_path + "/" + str(data_object_name) + collection_path = self.home + "/" + str(subcollection_name) + data_object_path = collection_path + "/" + str(data_object_name) - try: - self.session.data_objects.get(data_object_path) - return True - except (DataObjectDoesNotExist, CollectionDoesNotExist): - log.debug("Collection or data object (%s) does not exist", data_object_path) - return False - except NetworkException as e: - log.exception(e) - return False + try: + self.session.data_objects.get(data_object_path) + return True + except (DataObjectDoesNotExist, CollectionDoesNotExist): + log.debug("Collection or data object (%s) does not exist", data_object_path) + return False + except NetworkException as e: + log.exception(e) + return False def _in_cache(self, rel_path): """ Check if the given dataset is in the local cache and return True if so. """ @@ -355,36 +353,36 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): return file_ok def _download(self, rel_path): - log.debug("Pulling data object '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) + log.debug("Pulling data object '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) - p = Path(rel_path) - data_object_name = p.stem + p.suffix - subcollection_name = p.parent + p = Path(rel_path) + data_object_name = p.stem + p.suffix + subcollection_name = p.parent - collection_path = self.home + "/" + str(subcollection_name) - data_object_path = collection_path + "/" + str(data_object_name) - data_obj = None + collection_path = self.home + "/" + str(subcollection_name) + data_object_path = collection_path + "/" + str(data_object_name) + data_obj = None - try: - data_obj = self.session.data_objects.get(data_object_path) - except (DataObjectDoesNotExist, CollectionDoesNotExist): - log.warn("Collection or data object (%s) does not exist", data_object_path) - return False - except NetworkException as e: - log.exception(e) - return False + try: + data_obj = self.session.data_objects.get(data_object_path) + except (DataObjectDoesNotExist, CollectionDoesNotExist): + log.warn("Collection or data object (%s) does not exist", data_object_path) + return False + except NetworkException as e: + log.exception(e) + return False - if self.cache_size > 0 and data_obj.__sizeof__() > self.cache_size: - log.critical("File %s is larger (%s) than the cache size (%s). Cannot download.", - rel_path, data_obj.__sizeof__(), self.cache_size) - return False + if self.cache_size > 0 and data_obj.__sizeof__() > self.cache_size: + log.critical("File %s is larger (%s) than the cache size (%s). Cannot download.", + rel_path, data_obj.__sizeof__(), self.cache_size) + return False - log.debug("Pulled data object '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) + log.debug("Pulled data object '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) - with data_obj.open('r') as data_obj_fp, open(self._get_cache_path(rel_path), "wb") as cache_fp: - for chunk in iter(partial(data_obj_fp.read, CHUNK_SIZE), b''): - cache_fp.write(chunk) - return True + with data_obj.open('r') as data_obj_fp, open(self._get_cache_path(rel_path), "wb") as cache_fp: + for chunk in iter(partial(data_obj_fp.read, CHUNK_SIZE), b''): + cache_fp.write(chunk) + return True def _push_to_irods(self, rel_path, source_file=None, from_string=None): """ From 152ba9d6c09960c5a102d4eb9ac7bd95c88fcd30 Mon Sep 17 00:00:00 2001 From: Marius van den Beek Date: Sat, 17 Oct 2020 11:08:36 +0200 Subject: [PATCH 4/7] Update lib/galaxy/dependencies/conditional-requirements.txt --- lib/galaxy/dependencies/conditional-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/dependencies/conditional-requirements.txt b/lib/galaxy/dependencies/conditional-requirements.txt index fdf9822d862..deff3534097 100644 --- a/lib/galaxy/dependencies/conditional-requirements.txt +++ b/lib/galaxy/dependencies/conditional-requirements.txt @@ -9,7 +9,7 @@ statsd docker azure-storage==0.32.0 # python-irodsclient==0.8.3 -# git+https://github.com/kxk302/python-irodsclient.git +git+https://github.com/kxk302/python-irodsclient.git python-ldap==3.2.0 python-pam galaxycloudrunner From bffc81d456a45edee028d6a33029e8001cc019d7 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 17 Oct 2020 11:34:45 +0200 Subject: [PATCH 5/7] Use PEP508 convention for python-irods requirement --- lib/galaxy/dependencies/conditional-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/dependencies/conditional-requirements.txt b/lib/galaxy/dependencies/conditional-requirements.txt index deff3534097..d6f02509606 100644 --- a/lib/galaxy/dependencies/conditional-requirements.txt +++ b/lib/galaxy/dependencies/conditional-requirements.txt @@ -9,7 +9,7 @@ statsd docker azure-storage==0.32.0 # python-irodsclient==0.8.3 -git+https://github.com/kxk302/python-irodsclient.git +python-irodsclient@https://github.com/kxk302/python-irodsclient/archive/master.zip python-ldap==3.2.0 python-pam galaxycloudrunner From 69779ca401555098d5bdf0bf13f04c9e7b2731b8 Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Mon, 19 Oct 2020 10:59:39 -0400 Subject: [PATCH 6/7] Revised irods.py per code review. --- lib/galaxy/objectstore/irods.py | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/lib/galaxy/objectstore/irods.py b/lib/galaxy/objectstore/irods.py index 7c8938835e8..ac2fcc9263c 100644 --- a/lib/galaxy/objectstore/irods.py +++ b/lib/galaxy/objectstore/irods.py @@ -209,7 +209,13 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): raise Exception(IRODS_IMPORT_MESSAGE) self.home = "/" + self.zone + "/home/" + self.username - self._initialize() + + if irods is None: + raise Exception(IRODS_IMPORT_MESSAGE) + + self.session = iRODSSession(host=self.host, port=self.port, user=self.username, password=self.password, zone=self.zone) + # Set connection timeout + self.session.connection_timeout = self.timeout log.debug("irods __init__ %s", reload_timer) def shutdown(self): @@ -220,29 +226,6 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): except OSError: pass - def _initialize(self): - if irods is None: - raise Exception(IRODS_IMPORT_MESSAGE) - - self.session = self._configure_connection(host=self.host, port=self.port, user=self.username, password=self.password, zone=self.zone, timeout=self.timeout, poolsize=self.poolsize) - - def _configure_connection(self, host='localhost', port='1247', user='rods', password='rods', zone='tempZone', timeout=30, poolsize=1): - session = iRODSSession(host=host, port=port, user=user, password=password, zone=zone) - # Set connection timeout - session.connection_timeout = timeout - # Throws NetworkException if connection fails - try: - # We will create as many conections as poolsize. - # After creating the connection, release it - # so it goes into the idle connection queue - for idx in range(poolsize): - conn = session.pool.get_connection() - session.pool.release_connection(conn) - except NetworkException as e: - log.error('Could not create iRODS session: ' + str(e)) - raise - return session - @classmethod def parse_xml(cls, config_xml): return parse_config_xml(config_xml) From 58e969d352593385f9d4d7e1130d7fb2cb146f85 Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Mon, 19 Oct 2020 17:27:01 -0400 Subject: [PATCH 7/7] Using python_irodsclient version 0.8.4 to have connection refresh capability --- lib/galaxy/dependencies/conditional-requirements.txt | 3 +-- .../dependencies/pipfiles/default/pinned-dev-requirements.txt | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/dependencies/conditional-requirements.txt b/lib/galaxy/dependencies/conditional-requirements.txt index d6f02509606..a04613c5f7a 100644 --- a/lib/galaxy/dependencies/conditional-requirements.txt +++ b/lib/galaxy/dependencies/conditional-requirements.txt @@ -8,8 +8,7 @@ drmaa statsd docker azure-storage==0.32.0 -# python-irodsclient==0.8.3 -python-irodsclient@https://github.com/kxk302/python-irodsclient/archive/master.zip +python-irodsclient==0.8.4 python-ldap==3.2.0 python-pam galaxycloudrunner diff --git a/lib/galaxy/dependencies/pipfiles/default/pinned-dev-requirements.txt b/lib/galaxy/dependencies/pipfiles/default/pinned-dev-requirements.txt index ab2977c1e05..478151d33b0 100644 --- a/lib/galaxy/dependencies/pipfiles/default/pinned-dev-requirements.txt +++ b/lib/galaxy/dependencies/pipfiles/default/pinned-dev-requirements.txt @@ -41,8 +41,7 @@ pytest-mock==3.1.1 pytest-postgresql==2.3.0 pytest-pythonpath==0.7.3 pytest==5.4.3 -# python-irodsclient==0.8.3 -git+https://github.com/kxk302/python-irodsclient.git +python-irodsclient==0.8.4 pytz==2020.1 recommonmark==0.6.0 requests==2.24.0