Wrong name for CollectionMeta id

[mstfdkmn]

It seems like there’s a minor issue with the column ID name in the following lines.

https://github.com/irods/python-irodsclient/blob/main/irods/models.py#L144 (should be COL_META_COLL_ATTR_ID) and L154 (COL_META_RESC_ATTR_ID)

[korydraughn]

Nice catch. We'll get that fixed for the next release.

[trel]

11.5 year old bug!

e931fab

[korydraughn]

The following lines:

python-irodsclient/irods/models.py

Line 144 in f2394e0

id = Column(String, 'COL_META_COLL_ATTR_UNITS', 613)

python-irodsclient/irods/models.py

Line 154 in f2394e0

id = Column(String, 'COL_META_RESC_ATTR_UNITS', 633)

should be:

-   id = Column(String, 'COL_META_COLL_ATTR_UNITS', 613)
+   id = Column(String, 'COL_META_COLL_ATTR_ID', 613)

-   id = Column(String, 'COL_META_RESC_ATTR_UNITS', 633)
+   id = Column(String, 'COL_META_RESC_ATTR_ID', 633)

Based on the following:
https://github.com/irods/irods/blob/8e51ce51941615302852f1ef4b27679c4b0cab8e/lib/core/include/irods/rodsGenQuery.h#L201
https://github.com/irods/irods/blob/8e51ce51941615302852f1ef4b27679c4b0cab8e/lib/core/include/irods/rodsGenQuery.h#L217

[trel]

And we'll need a test to validate these (and perhaps others).

[d-w-moore]

That's a good catch. I'm not convinced there's an easy way to test, though, unless we assert its compabitility with other clients (like imeta).

[d-w-moore]

Oh . i see, not a swap of two fields in the same row, but an omission in effect in two different tables. Wow, how did we not catch it before?

[trel]

Nobody's used that field in their own code before.

[d-w-moore]

Ok. And I see that we can test by extracting the dict keys from query results labelled with COL_META_COLL_ATTR_UNITS to make sure they (all) have an icat_id of 612. After a little experimentation. But that won't get any possible other errors we might've made in our mapping.

[d-w-moore]

$ python meta_coll_query.py 
[{<irods.column.Column 615 COL_META_COLL_MODIFY_TIME>: datetime.datetime(2024, 10, 21, 14, 17, 33),
  <irods.column.Column 614 COL_META_COLL_CREATE_TIME>: datetime.datetime(2024, 10, 21, 14, 17, 33),
  <irods.column.Column 612 COL_META_COLL_ATTR_UNITS>: 'c',
  <irods.column.Column 611 COL_META_COLL_ATTR_VALUE>: 'b',
  <irods.column.Column 610 COL_META_COLL_ATTR_NAME>: 'a',
  <irods.column.Column 613 COL_META_COLL_ATTR_UNITS>: '28000'}]

source:

#!/usr/bin/python3
from irods.models import CollectionMeta
from irods.helpers import make_session
s = make_session()
a = list(s.query(CollectionMeta))
import pprint
pprint.pprint(a)

[d-w-moore]

Wrote this script as a basis for a test - it compares the loaded columns in PRC's database model classes against a copy of rodsGenQuery.h

import re
r=re.compile(r'\s*#\s*define\s\s*(?P<column_name>COL_\w+)\s+(?P<column_value>[0-9]+)\s*')  
fn = 'rodsGenQuery.h'
l=open(fn,'r').readlines()
column_name_to_integer = dict((m.group('column_name'),m.group('column_value')) for m in (r.match(ln) for ln in l) if m)
import irods.models

for n,col in irods.models.ModelBase.columns.items():
    mne = col.icat_key
    num = col.icat_id

    if n == 0:
        continue
    assert n == num

    num_from_irods_header_lookup = column_name_to_integer['COL_'+mne if not mne.startswith('COL_')
        else mne]

    # The check:
    if num_from_irods_header_lookup == str(num):
        print('y',end='')
    else:
        print(f'\n a column named {mne} had {num} instead of '
              f'{num_from_irods_header_lookup}')

Ran it a couple times, found some things :

$ (cd ~;python check_column_numeric_keys_against_header.py )
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
 a column named COL_META_COLL_ATTR_UNITS had 613 instead of 612
yyyyy
 a column named COL_META_RESC_ATTR_UNITS had 633 instead of 632
yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyTraceback (most recent call last):
  File "./tmp.py", line 20, in <module>
    else mne]
KeyError: 'COL_TICKET_ALLOWED_USER'

So in other words, this turned up not only the wrongly assigned UNITS fields noted in @korydraughn's above issue comment but also some misnamed column names COL_TICKET_ALLOWED_USER and COL_TICKET_ALLOWED_GROUP (both need _NAME appended to the end if we want them to agree with the iRODS header file).

[d-w-moore]

Ran again with those column corrections to the PRC source, and got all "y" (that is, passing, i.e. the testing script's code for "yes", meaning the loaded column named checked out as correct against the rodsGenQuery.h header).
Sorry for the cryptic output. But it will all be converted to a test function anyway...

[trel]

nice. so we'll see a diff and can decide if those other two are what we want. good find.