IRODS_QUERY_LIMIT now settable/gettable under Python client settings #715
Conversation
| @@ -573,6 +573,12 @@ certain environment variables: | |||
| - Default Value: `False` | |||
| - Environment Variable Override: `PYTHON_IRODSCLIENT_CONFIG__LEGACY_AUTH__PAM__FORCE_USE_OF_DEDICATED_PAM_API` | |||
|
|
|||
| - Setting: Default number of rows for a Query to retrieve by default. | |||
There was a problem hiding this comment.
I think we could use the word "limit" or "maximum" in this description.
Also, I'm a little confused by the word "default" here. This is a configuration value to affect the maximum number of rows that a query can retrieve at a time, right?
There was a problem hiding this comment.
Admittedly, that description seems wrong, so yes let's change it.
I wasn't actually that familiar with the query limit and its function, so I needed to reconsider this.
| @@ -36,7 +36,8 @@ | |||
| "SELECT_COUNT": 6, | |||
| } | |||
|
|
|||
| IRODS_QUERY_LIMIT = os.getenv("IRODS_QUERY_LIMIT", 500) | |||
| IRODS_QUERY_LIMIT = 500 | |||
There was a problem hiding this comment.
IRODS_QUERY_LIMIT is used on L200. That means the number is always 500 now. Should we replace that with a call to the irods_query_limit getter?
There was a problem hiding this comment.
Not sure if using the getter would yield any advantage ...?
IRODS_QUERY_LIMIT will be altered through the configuration setting.
Unfortunately, or fortunately, there are no const variables in Python!
There was a problem hiding this comment.
Ohhh, I see. Okay, let's at least leave a comment saying that the value is changed by that setting
| def irods_query_limit(self, int_value): | ||
| import irods.query | ||
| irods.query.IRODS_QUERY_LIMIT = int(int_value) | ||
|
|
||
|
|
||
| connections = ConnectionsProperties() |
There was a problem hiding this comment.
Just curious if ConnectionsProperties is the best place for this. Would it be appropriate to introduce a QueryProperties?
There was a problem hiding this comment.
Having a QueryProperties type makes the most sense to me, but it will ultimately depend on how it fits into the existing query interface.
|
we need an issue number for this work? edit: found it #712 |
Yes |
No description provided.