rosshamish / classtime

university schedule generation and course data API
MIT License
16 stars 2 forks source link

Handle multiple ldap attribute values #81

Open rosshamish opened 9 years ago

rosshamish commented 9 years ago

So I wrote this list comprehension to extract ldap result data forever ago

return [{k:v[0].decode('utf-8')
        for k, v in d[1].items()}
        for d in ldap_result_data]

Where

ldap_result_data = [
    [?, {"asString": ["ANTHR 111"], "credits": ["3"], ... }],
    [?, {"asString": ["BLAW 222"], "credits": ["5"],  ... }],
    ...
    <more 2-element lists, where we don't use the first element>
]

And by doing v[0], it assumes that all value lists have just a single element, and ignores the possibility that an attribute could have multiple values.

This has been working fine, clearly.

However, I decided to check it out to see if that assumption was true. I added some logging:

logger = logging.getLogger()
errormsg = 'ldap result with more than one attribute val: {}'
for d in data:
    for k,v in d[1].items():
        if len(v) > 1:
            logger.critical(errormsg.format(v))

return [{k:v[0].decode('utf-8')
        for k, v in d[1].items()}
        for d in ldap_result_data]

, then started up the server and hit it with a random schedule request.

Here are the server logs:

INFO:   Received schedule request       07:47:13  [classtime.scheduling.schedule_generator]
CRITICAL:   ldap result with more than one attribute val: ['ddonald', 'ckw', 'evelyn', 'rss5', 'velvalee']      07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['ddonald', 'ckw', 'evelyn', 'rss5', 'velvalee']      07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['pheasant', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['cymartin', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['dunningl', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['felliott', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['underwoo', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['clouis', 'aw1', 'jrs6']     07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['ledge', 'aw1', 'jrs6']      07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['vansomer', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['cymartin', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['felliott', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['sla3', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['rbourque', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['annaleah', 'aw1', 'jrs6']       07:47:15  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['ledge', 'aw1', 'jrs6']      07:47:15  [classtime.remote_db.ldapdb]

They look like CCIDs! They must the the profs of sections (the "instructor" attribute).

I logged onto my @ualberta.ca gmail, and looked them up by composing a draft and typing the CCID in the To: field. They're all definitely valid - even the second and third ones. I also googled the names of some - they are, in fact, uAlberta profs.

Fixing this would require changing the "instructor" column in the database to a list - but SQL doesn't support lists, so this will need to be a "relationship". Relationships are JOINs between tables, which means that a new "instructor" table will be necessary.

Essentially, it won't be hard, but it won't be trivial either.

So here're the questions:

the questions

Are we strongly considering ratemyprof integration? If so, this is probably worth doing.

Can you think of a representative course with multiple profs? We could use that to determine what it "means" for a course to have multiple professors.

rosshamish commented 9 years ago

Scratch question two: Cmput 274 fits the bill.

Here's the logs from fetching Cmput 274 sections:

INFO:   Received schedule request       08:21:48  [classtime.scheduling.schedule_generator]
CRITICAL:   ldap result with more than one attribute val: ['hackman', 'mbowling', 'szepesva', 'trdavis1', 'tayee', 'xhu3']      08:21:49  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['hackman', 'mbowling', 'szepesva', 'trdavis1', 'tayee', 'xhu3']      08:21:49  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['hackman', 'mbowling', 'szepesva', 'trdavis1', 'tayee', 'xhu3']      08:21:49  [classtime.remote_db.ldapdb]
CRITICAL:   ldap result with more than one attribute val: ['hackman', 'mbowling', 'szepesva', 'trdavis1', 'tayee', 'xhu3']      08:21:49  [classtime.remote_db.ldapdb]

Unless Leah is the main instructor this semester (Bowling was for me), then this means that the first professor in the list is not necessarily the main instructor. Nooo!

ahoskins commented 9 years ago

A course like CMPUT 274 would likely have multiple prof's in the DB. And in this case, it's not just a matter of semantics to say "multiple prof's", but it actually uses multiple prof's.

Something somewhat interesting is that beartracks doesn't say multiple prof's (as far as I know). For instance, 274 I know only said one of the prof's. So then if beartracks doesn't display them all, then maybe we shouldn't either.

So I don't really know. But if beartracks doesn't, I think it might make sense for us not to as well.

ahoskins commented 9 years ago

On my beartracks this year I'm pretty sure I said scaba was the instructor