[#712] irods query limit as part of irods.client_configuration

Note that as a configuration setting, it may optionally be loaded from
an environment variable.

Author: d-w-moore

README.md

@@ -543,7 +543,19 @@ loading of the settings from the settings file, assuming it exists.
 (Failure to find the file at the indicated path will be logged as a
 warning.)
 
-Settings can also be saved and loaded manually using the `save()` and
+If the process of loading configuration settings generates an unhandled exception,
+the `irods` module will consult the value of the environment variable
+`PYTHON_IRODSCLIENT_CONFIGURATION_LOAD_ERRORS_FATAL` and either (if `True`) incur
+a fatal error or (if `False`) simply log it as a warning.  By default this
+environment variable is taken as `False` if not set, although that is likely to
+change to `True` in the future.
+This decision indicates a desire by the client library's designers to flag such
+errors as fatal by aborting the process if possible, rather than aquiescing silently
+to an unintended consequence.  It is recommended that all client library users
+set this environment variable to `True` if their applications load settings
+from the environment or configuration files.
+
+Settings can be saved and loaded manually using the `save()` and
 `load()` functions in the `irods.client_configuration`
 module. Each of these functions accepts an optional `file`
 parameter which, if set to a non-empty string, will override the
@@ -609,6 +621,12 @@ certain environment variables:
     - Default Value: `False`
     - Environment Variable Override: `PYTHON_IRODSCLIENT_CONFIG__LEGACY_AUTH__PAM__FORCE_USE_OF_DEDICATED_PAM_API`
 
+-   Setting: The maximum number of rows a Query iteration will retrieve.  Can be overridden using Query's `limit` method. This is synonymous with the variable `irods.query.IRODS_QUERY_LIMIT`, and a read (or write) of this configuration setting will read (or affect) that variable.
+    -   Dotted Name: `genquery1.irods_query_limit`
+    -   Type: `int`
+    -   Default Value: 500 (The library's traditional maximum batch row count, also stored as `irods.query._IRODS_QUERY_LIMIT`)
+    -   Environment Variable Override: `PYTHON_IRODSCLIENT_CONFIG__GENQUERY1__IRODS_QUERY_LIMIT`
+
 -   Setting: Default choice of XML parser for all new threads.
     -   Dotted Name: `connections.xml_parser_default`
     -   Type: `str`
@@ -1176,8 +1194,8 @@ Also, core.py rules may only be run directly by a rodsadmin, currently.
 [See this issue for discussion](https://github.com/irods/irods_rule_engine_plugin_python/issues/105).
 
 
-General Queries
----------------
+GenQuery1 Queries
+-----------------
 
 A session object's `query` method accepts a list of columns and models to be included in any returned rows.
 
@@ -1285,6 +1303,25 @@ parameter to the query:
 +---------------------+
 ```
 
+If the user desires a smaller paging size, this can be accomplished by changing
+the `genquery1.irods_query_limit` configuration setting.  This directly changes
+the `irods.query.IRODS_QUERY_LIMIT` value and therefore affects Query objects'
+`all` and `get_batches` methods.
+
+Note, however, that expressions such as `list(Query(...))` and `(row for row in
+Query)` are not affected by this setting.
+
+The setting may be given any positive integer value.  Attempting to set it to
+zero or a negative number will not affect the value of `IRODS_QUERY_LIMIT` but will
+raise a `ConfigurationValueError` if done in the course of a running iRODS
+client application.  Importantly, if the invalid setting is attempted by a loading
+configuration, the otherwise fatal error could be absorbed into a warning-level log action,
+depending on the value of the `PYTHON_IRODSCLIENT_CONFIGURATION_LOAD_ERRORS_FATAL`
+environment variable.  See: [Python iRODS Client Settings File](#python-irods-client-settings-file).
+
+A copy of the original value of `IRODS_QUERY_LIMIT` (before overriding values are loaded from the
+configuration) is stored within `irods.query.cached_values.IRODS_QUERY_LIMIT`.
+
 Specific Queries
 ----------------
 

irods/client_configuration/__init__.py

@@ -12,7 +12,7 @@
 # Duplicate here for convenience
 from .. import DEFAULT_CONFIG_PATH
 
-logger = logging.Logger(__name__)
+logger = logging.getLogger(__name__)
 
 
 class iRODSConfiguration:
@@ -57,9 +57,31 @@ def xml_parser_default(self, str_value):
 
         return set_default_XML_by_name(str_value)
 
-
 connections = ConnectionsProperties()
 
+class ConfigurationError(BaseException): pass
+class ConfigurationValueError(ValueError,ConfigurationError): pass
+
+class Genquery1_Properties(iRODSConfiguration, metaclass=iRODSConfigAliasMetaclass):
+
+    @property
+    def irods_query_limit(self):
+        import irods.query
+        return irods.query.IRODS_QUERY_LIMIT
+
+    @irods_query_limit.setter
+    def irods_query_limit(self, target_value):
+        import irods.query
+        requested = int(target_value)
+
+        if requested <= 0:
+            raise ConfigurationValueError(f'Error setting IRODS_QUERY_LIMIT to [{requested}]. Use positive values only.')
+
+        irods.query.IRODS_QUERY_LIMIT = requested
+
+genquery1 = Genquery1_Properties()
+
+
 # #############################################################################
 #
 # Classes for building client configuration categories

irods/helpers/__init__.py

@@ -15,7 +15,6 @@
     "get_data_object",
 ]
 
-
 class StopTestsException(Exception):
 
     def __init__(self, *args, **kwargs):
@@ -144,3 +143,16 @@ def get_collection(sess, logical_path):
         return sess.collections.get(logical_path)
     except ex.CollectionDoesNotExist:
         return None
+
+
+# Utility class and factory function for storing the original value of variables within the given namespace.
+def create_value_cache(namespace:dict):
+    class CachedValues:
+        __namespace = namespace
+
+        @classmethod
+        def make_entry(cls, name):
+            cached_value = cls.__namespace[name]
+            setattr(cls,name,property(lambda self: cached_value))
+
+    return CachedValues()

irods/manager/access_manager.py

@@ -73,7 +73,7 @@ def get(self, target, report_raw_acls=True, **kw):
         results = (
             self.sess.query(user_type.name, user_type.zone, access_type.name)
             .filter(*conditions)
-            .all()
+            ._all()
         )
 
         def get_usertype(row):

irods/manager/metadata_manager.py

@@ -83,7 +83,7 @@ def get(self, model_cls, path):
         columns = (model.id, model.name, model.value, model.units)
         if self.use_timestamps:
             columns += (model.create_time, model.modify_time)
-        results = self.sess.query(*columns).filter(*conditions).all()
+        results = self.sess.query(*columns).filter(*conditions)._all()
 
         def meta_opts(row):
             opts = {"avu_id": row[model.id]}

irods/query.py

@@ -1,5 +1,3 @@
-import os
-
 from collections import OrderedDict
 
 from irods import MAX_SQL_ROWS
@@ -36,7 +34,8 @@
     "SELECT_COUNT": 6,
 }
 
-IRODS_QUERY_LIMIT = os.getenv("IRODS_QUERY_LIMIT", 500)
+IRODS_QUERY_LIMIT = 500
+
 class Query:
 
     def __init__(self, sess, *args, **kwargs):
@@ -198,7 +197,7 @@ def _kw_message(self):
         return StringStringMap(dct)
 
     def _message(self):
-        max_rows = IRODS_QUERY_LIMIT if self._limit == -1 else self._limit
+        max_rows = IRODS_QUERY_LIMIT if self._limit < 0 else self._limit
         args = {
             "maxRows": max_rows,
             "continueInx": self._continue_index,
@@ -233,6 +232,13 @@ def close(self):
         """
         self.limit(0).execute()
 
+    def _all(self):
+        """Internally used version of all().  Unlike the public version, the returned iterator when
+        executed to completion is unaffected by IRODS_QUERY_LIMIT.  This is in order to accurately
+        reflect the enumeration of sub-objects, e.g. AVU's associated with an object.
+        """
+        return self.get_results()
+
     def all(self):
         result_set = self.execute()
         if result_set.continue_index > 0:
@@ -382,3 +388,9 @@ def get_results(self):
         for result_set in self.get_batches():
             for result in result_set:
                 yield result
+
+
+# Record a copy of the original value of IRODS_QUERY_LIMIT for possible future access.
+import irods.helpers as helpers
+cached_values = helpers.create_value_cache(globals())
+cached_values.make_entry('IRODS_QUERY_LIMIT')

irods/test/query_test.py

@@ -26,14 +26,15 @@
     CAT_UNKNOWN_SPECIFIC_QUERY,
     CAT_INVALID_ARGUMENT,
 )
-from irods.query import SpecificQuery
+from irods import MAX_SQL_ROWS
+import irods.client_configuration as config
 from irods.column import Like, NotLike, Between, In
+import irods.keywords as kw
 from irods.meta import iRODSMeta
+from irods.query import SpecificQuery
 from irods.rule import Rule
-from irods import MAX_SQL_ROWS
-from irods.test.helpers import irods_shared_reg_resc_vault
 import irods.test.helpers as helpers
-import irods.keywords as kw
+from irods.test.helpers import irods_shared_reg_resc_vault
 
 IRODS_STATEMENT_TABLE_SIZE = 50
 
@@ -1048,6 +1049,30 @@ def test_negating_columns_in_genquery1_results__issue_755(self):
             row = list(q.all())[0]
             self.assertEqual(columns_to_negate & row.keys(), intersect)
 
+    def test_set_query_limit__issue_712(self):
+        # Test requires that 0 < num_limited_results < num_total_objects.
+        num_limited_results = 4
+        num_total_objects = num_limited_results + 6
+        data_objs = []
+        try:
+            # Create a number of data objects.
+            for name in range(num_total_objects):
+                data_objs.append(self.sess.data_objects.create(f'{self.coll_path}/issue_712_obj{name}'))
+
+            # Test a query limit via configuration.
+            with config.loadlines(
+                entries=[dict(setting="genquery1.irods_query_limit", value=num_limited_results)]
+            ):
+                limited_results = list(self.sess.query(DataObject.id).filter(Like(DataObject.name, '%issue_712_obj%')).all())
+                self.assertEqual(num_limited_results, len(limited_results))
+
+            # Test the query limit is no longer in effect.
+            non_limited_results = list(self.sess.query(DataObject.id).filter(Like(DataObject.name, '%issue_712_obj%')).all())
+            self.assertEqual(num_total_objects, len(non_limited_results))
+        finally:
+            for d in data_objs:
+                d.unlink(force=True)
+
 
 class TestSpecificQuery(unittest.TestCase):