Skip to content
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
2b65404
fix conftest to comply to exception implementation in sarc/ldap/super…
nurbal Feb 23, 2024
9433bcc
conftest exception for delegations and supervisors overrides
nurbal Feb 23, 2024
0b5763b
test for supervisors overrides
nurbal Feb 23, 2024
8b5d2b8
supervisors override implementation
nurbal Feb 23, 2024
9f0f418
add teacher_delegations in User API
nurbal Feb 23, 2024
c69a338
add test for teacher_delegations + lint
nurbal Feb 23, 2024
e7f3393
remove print
nurbal Feb 23, 2024
2418574
add teachers delegations implementation
nurbal Feb 23, 2024
abc367b
fix tests made without exceptions file
nurbal Feb 23, 2024
f206155
lint
nurbal Feb 23, 2024
10a720a
fix test_query_to_ldap_server_and_writing_to_output_json (ignore co_s…
nurbal Mar 1, 2024
8cef544
Merge commit 'be44d1a5f2ee0ed5bac764009e8d3ed87518cd2a' into users_ex…
nurbal Mar 1, 2024
9d61cb4
updated test_acquire_users_supervisors and test_acquire_users_co_supe…
nurbal Mar 1, 2024
7469517
lint/black/whatever
nurbal Mar 1, 2024
a95b9e7
Merge branch 'master' into users_exceptions
nurbal Apr 11, 2024
627f9d6
lint
nurbal Apr 11, 2024
9249978
Move the supervisors override and prof delegation exceptions to sarc.…
nurbal Apr 16, 2024
de015ae
Merge branch 'master' into users_exceptions
nurbal Apr 16, 2024
ad9f863
Merge branch 'master' into users_exceptions
nurbal Apr 18, 2024
56eb39c
fix test `test_load_job_series_with_users` to include the new `teache…
nurbal Apr 18, 2024
0329790
Merge branch 'master' into users_exceptions
nurbal Apr 23, 2024
8644834
Merge commit 'bab50a574ce25cf52ccf45fcd8aa4638f14da6c0' into users_ex…
nurbal Sep 21, 2024
a32536f
lint
nurbal Sep 25, 2024
ce1523d
fix tests: `test_get_user` and `test_get_users`
nurbal Sep 30, 2024
b945460
update lint exceptions ( disable too-many-positional-arguments )
nurbal Sep 30, 2024
59cab5f
fix test `test_acquire_ldap_revision_change`
nurbal Oct 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions sarc/ldap/acquire.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
from sarc.ldap.mymila import fetch_mymila
from sarc.ldap.read_mila_ldap import fetch_ldap
from sarc.ldap.revision import commit_matches_to_database
from sarc.ldap.users_exceptions import (
apply_users_delegation_exceptions,
apply_users_supervisor_exceptions,
)
from sarc.traces import using_trace


Expand Down Expand Up @@ -114,6 +118,12 @@ def run(prompt=False):
for _, user in DD_persons_matched.items():
fill_computed_fields(user)

# apply delegation exceptions
apply_users_delegation_exceptions(DD_persons_matched, cfg.ldap, span)

# apply supervisor exceptions
apply_users_supervisor_exceptions(DD_persons_matched, cfg.ldap, span)

# These associations can now be propagated to the database.
span.add_event("Committing matches to database ...")
commit_matches_to_database(
Expand Down
2 changes: 2 additions & 0 deletions sarc/ldap/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class User(BaseModel):
mila: Credentials
drac: Optional[Credentials]

teacher_delegations: Optional[list[str]] = None

mila_ldap: dict
drac_members: Optional[dict]
drac_roles: Optional[dict]
Expand Down
1 change: 1 addition & 0 deletions sarc/ldap/revision.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ def user_insert(newuser: dict) -> list:
"mila",
"drac_roles",
"drac_members",
"teacher_delegations",
)

update = {
Expand Down
1 change: 1 addition & 0 deletions sarc/ldap/supervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ def sortkey(x):
return sorted(supervisors, key=sortkey, reverse=True)


# pylint: disable=too-many-branches
def resolve_supervisors(
ldap_people: list[dict], group_to_prof: dict, exceptions: dict
) -> SupervisorMatchingErrors:
Expand Down
56 changes: 56 additions & 0 deletions sarc/ldap/users_exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
from sarc.config import LDAPConfig
from sarc.ldap.read_mila_ldap import load_ldap_exceptions


def apply_users_delegation_exceptions(DD_persons, ldap_config: LDAPConfig, span):
"""
Apply manual exceptions to users;
these exceptions are defined in the exceptions.json file refered in the LDAPConfig.
"""
span.add_event("Applying users delegation exceptions ...")
# Load exceptions
exceptions = load_ldap_exceptions(ldap_config)

for _, user in DD_persons.items():
if (
exceptions
and user["mila_ldap"]["mila_email_username"] in exceptions["delegations"]
):
span.add_event(
f"Applying delegation exception for {user['mila_ldap']['mila_email_username']} ..."
)
user["teacher_delegations"] = exceptions["delegations"][
user["mila_ldap"]["mila_email_username"]
]


def apply_users_supervisor_exceptions(DD_persons, ldap_config: LDAPConfig, span):
"""
Apply manual exceptions to users;
these exceptions are defined in the exceptions.json file refered in the LDAPConfig.
"""
span.add_event("Applying users supervisor exceptions ...")
# Load exceptions
exceptions = load_ldap_exceptions(ldap_config)

for _, user in DD_persons.items():
# if there is a supervisors override, use it whatever the student status may be
if exceptions and user["mila_ldap"]["mila_email_username"] in exceptions.get(
"supervisors_overrides", []
):
span.add_event(
f"Applying supervisor exception for {user['mila_ldap']['mila_email_username']} ..."
)
supervisors = exceptions["supervisors_overrides"][
user["mila_ldap"]["mila_email_username"]
]
user["mila_ldap"]["supervisor"] = None
user["mila_ldap"]["co_supervisor"] = None
if len(supervisors) >= 1:
user["mila_ldap"]["supervisor"] = supervisors[0]
else:
user["mila_ldap"]["supervisor"] = None
if len(supervisors) >= 2:
user["mila_ldap"]["co_supervisor"] = supervisors[1]
else:
user["mila_ldap"]["co_supervisor"] = None
20 changes: 18 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,24 @@ def file_contents():
"""
exceptions_json_path = """
{
"not_prof": [],
"not_student": []
"not_teacher": [],
"not_student": [],
"delegations": {
"[email protected]": [
"[email protected]",
"[email protected]"
]
},
"supervisors_overrides": {
"[email protected]": [
"[email protected]"
],
"[email protected]": [
"[email protected]",
"[email protected]"
]
}

}
"""

Expand Down
93 changes: 80 additions & 13 deletions tests/functional/cli/acquire/test_acquire_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,37 @@ def mock_query_ldap(
js_user = get_user(drac_account_username="stranger.person")
assert js_user is None

# test supervisor overrides
js_user = get_user(mila_email_username="[email protected]")
assert js_user is not None
assert js_user.mila_ldap["supervisor"] == "[email protected]"
assert js_user.mila_ldap["co_supervisor"] == None

js_user = get_user(mila_email_username="[email protected]")
assert js_user is not None
assert js_user.mila_ldap["supervisor"] == "[email protected]"
assert js_user.mila_ldap["co_supervisor"] == "[email protected]"

# test delegations
# john.smith003 should have delegations for john.smith004 and john.smith005
# john.smith004 should have no delegations
# john.smith005 should have no delegations

js_user = get_user(mila_email_username="[email protected]")
assert js_user is not None
assert js_user.teacher_delegations is not None
assert "[email protected]" in js_user.teacher_delegations
assert "[email protected]" in js_user.teacher_delegations
assert "[email protected]" not in js_user.teacher_delegations

js_user = get_user(mila_email_username="[email protected]")
assert js_user is not None
assert js_user.teacher_delegations == None

js_user = get_user(mila_email_username="[email protected]")
assert js_user is not None
assert js_user.teacher_delegations == None

# Check traces
# NB: We don't check logging here, because
# this execution won't display "acquire users" logs,
Expand All @@ -132,14 +163,28 @@ def mock_query_ldap(
assert len(spans) == 1
assert spans[0].name == "match_drac_to_mila_accounts"
assert spans[0].status.status_code == StatusCode.OK
assert len(spans[0].events) == 4
assert len(spans[0].events) == 9
assert (
spans[0].events[0].name
== "Loading mila_ldap, drac_roles and drac_members from files ..."
)
assert spans[0].events[1].name == "Loading matching config from file ..."
assert spans[0].events[2].name == "Matching DRAC/CC to mila accounts ..."
assert spans[0].events[3].name == "Committing matches to database ..."
assert spans[0].events[3].name == "Applying users delegation exceptions ..."
assert (
spans[0].events[4].name
== "Applying delegation exception for [email protected] ..."
)
assert spans[0].events[5].name == "Applying users supervisor exceptions ..."
assert (
spans[0].events[6].name
== "Applying supervisor exception for [email protected] ..."
)
assert (
spans[0].events[7].name
== "Applying supervisor exception for [email protected] ..."
)
assert spans[0].events[8].name == "Committing matches to database ..."


@pytest.mark.parametrize(
Expand Down Expand Up @@ -184,6 +229,10 @@ def test_acquire_users_supervisors(
nbr_users = 4
nbr_profs = 2

# for the test we will use the user with index 3,
# which is the first user who has no supervisor override in the mock data
# so that this test won't be affected by the previous test

# Mock the fake LDAP data used for the tests
def mock_query_ldap(
local_private_key_file, local_certificate_file, ldap_service_uri
Expand All @@ -192,7 +241,7 @@ def mock_query_ldap(
return fake_raw_ldap_data(
nbr_users,
hardcoded_values_by_user={
2: { # The first user who is not a prof is the one with index 2
3: { # The first user who is not a prof is the one with index 3
"supervisor": ldap_supervisor
}
},
Expand All @@ -206,7 +255,7 @@ def mock_query_mymila(tmp_json_path):
nbr_users=nbr_users,
nbr_profs=nbr_profs,
hardcoded_values_by_user={
2: { # The first user who is not a prof is the one with index 2
3: { # The first user who is not a prof is the one with index 3
"Supervisor Principal": mymila_supervisor
}
},
Expand All @@ -229,8 +278,8 @@ def mock_query_mymila(tmp_json_path):

# Validate the results of all of this by inspecting the database.
js_user = get_user(
mila_email_username=f"john.smith002@mila.quebec"
) # We modified the user with index 2; thus this is the one we retrieve
mila_email_username=f"john.smith003@mila.quebec"
) # We modified the user with index 3; thus this is the one we retrieve
assert js_user.mila_ldap["supervisor"] == expected_supervisor


Expand Down Expand Up @@ -276,6 +325,10 @@ def test_acquire_users_co_supervisors(
nbr_users = 4
nbr_profs = 2

# for the test we will use the user with index 3,
# which is the first user who has no supervisor override in the mock data
# so that this test won't be affected by the previous test

# Mock the fake LDAP data used for the tests
def mock_query_ldap(
local_private_key_file, local_certificate_file, ldap_service_uri
Expand All @@ -284,7 +337,7 @@ def mock_query_ldap(
return fake_raw_ldap_data(
nbr_users,
hardcoded_values_by_user={
2: { # The first user who is not a prof is the one with index 2
3: { # The first user who is not a prof is the one with index 3
"co_supervisor": ldap_co_supervisor
}
},
Expand All @@ -298,7 +351,7 @@ def mock_query_mymila(tmp_json_path):
nbr_users=nbr_users,
nbr_profs=nbr_profs,
hardcoded_values_by_user={
2: { # The first user who is not a prof is the one with index 2
3: { # The first user who is not a prof is the one with index 3
"Co-Supervisor": mymila_co_supervisor
}
},
Expand All @@ -321,8 +374,8 @@ def mock_query_mymila(tmp_json_path):

# Validate the results of all of this by inspecting the database.
js_user = get_user(
mila_email_username=f"john.smith002@mila.quebec"
) # We modified the user with index 2; thus this is the one we retrieve
mila_email_username=f"john.smith003@mila.quebec"
) # We modified the user with index 3; thus this is the one we retrieve
assert js_user.mila_ldap["co_supervisor"] == expected_co_supervisor


Expand Down Expand Up @@ -421,12 +474,26 @@ def mock_query_ldap(
assert len(spans) == 1
assert spans[0].name == "match_drac_to_mila_accounts"
assert spans[0].status.status_code == StatusCode.OK
assert len(spans[0].events) == 5
assert len(spans[0].events) == 10
assert (
spans[0].events[0].name
== "Loading mila_ldap, drac_roles and drac_members from files ..."
)
assert spans[0].events[1].name == "Loading matching config from file ..."
assert spans[0].events[2].name == "Matching DRAC/CC to mila accounts ..."
assert spans[0].events[3].name == "Committing matches to database ..."
assert spans[0].events[4].name == "Saving 1 manual matches ..."
assert spans[0].events[3].name == "Applying users delegation exceptions ..."
assert (
spans[0].events[4].name
== "Applying delegation exception for [email protected] ..."
)
assert spans[0].events[5].name == "Applying users supervisor exceptions ..."
assert (
spans[0].events[6].name
== "Applying supervisor exception for [email protected] ..."
)
assert (
spans[0].events[7].name
== "Applying supervisor exception for [email protected] ..."
)
assert spans[0].events[8].name == "Committing matches to database ..."
assert spans[0].events[9].name == "Saving 1 manual matches ..."
1 change: 1 addition & 0 deletions tests/functional/jobs/test_func_load_job_series.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
"user.name",
"user.record_start",
"user.record_end",
"user.teacher_delegations",
"user.mila.username",
"user.mila.email",
"user.mila.active",
Expand Down
Loading