Skip to content

Commit

Permalink
Merge branch 'master' into sarc-331
Browse files Browse the repository at this point in the history
  • Loading branch information
nurbal authored Oct 5, 2024
2 parents 505910a + ee50c58 commit f91b467
Show file tree
Hide file tree
Showing 16 changed files with 203 additions and 45 deletions.
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ disable = [
"import-outside-toplevel", # These imports are useful to reduce loading times
"too-many-arguments",
"too-many-locals",
"too-many-positional-arguments",
"missing-module-docstring",
"missing-class-docstring",
"missing-function-docstring",
Expand Down
1 change: 0 additions & 1 deletion sarc/alerts/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class CachedResult:

@dataclass(unsafe_hash=True)
class Timespan:

# Time duration
duration: timedelta

Expand Down
1 change: 0 additions & 1 deletion sarc/alerts/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ class HealthMonitorConfig:
checks: dict[str, TaggedSubclass[HealthCheck]] = field(default_factory=dict)

def __post_init__(self):

all_checks = {}

# Parameterize the checks
Expand Down
2 changes: 2 additions & 0 deletions sarc/client/users/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,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
10 changes: 10 additions & 0 deletions sarc/users/acquire.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
from sarc.users.mymila import fetch_mymila
from sarc.users.read_mila_ldap import fetch_ldap
from sarc.users.revision import commit_matches_to_database
from sarc.users.users_exceptions import (
apply_users_delegation_exceptions,
apply_users_supervisor_exceptions,
)


def run(
Expand Down Expand Up @@ -135,6 +139,12 @@ def run(
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
1 change: 1 addition & 0 deletions sarc/users/revision.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def user_insert(newuser: dict) -> list:
"mila",
"drac_roles",
"drac_members",
"teacher_delegations",
)

update = {
Expand Down
1 change: 1 addition & 0 deletions sarc/users/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/users/users_exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
from sarc.config import LDAPConfig
from sarc.users.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 @@ -118,6 +118,37 @@ def test_acquire_users(cli_main, patch_return_values, mock_file, captrace):
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 test_acquire_users(cli_main, patch_return_values, mock_file, captrace):

assert spans[1].name == "match_drac_to_mila_accounts"
assert spans[1].status.status_code == StatusCode.OK
assert len(spans[1].events) == 4
assert len(spans[1].events) == 9
assert (
spans[1].events[0].name
== "Loading mila_ldap, drac_roles and drac_members from files ..."
)
assert spans[1].events[1].name == "Loading matching config from file ..."
assert spans[1].events[2].name == "Matching DRAC/CC to mila accounts ..."
assert spans[1].events[3].name == "Committing matches to database ..."
assert spans[1].events[3].name == "Applying users delegation exceptions ..."
assert (
spans[1].events[4].name
== "Applying delegation exception for [email protected] ..."
)
assert spans[1].events[5].name == "Applying users supervisor exceptions ..."
assert (
spans[1].events[6].name
== "Applying supervisor exception for [email protected] ..."
)
assert (
spans[1].events[7].name
== "Applying supervisor exception for [email protected] ..."
)
assert spans[1].events[8].name == "Committing matches to database ..."


@pytest.mark.parametrize(
Expand Down Expand Up @@ -184,12 +229,16 @@ 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

patch_return_values(
{
"sarc.users.read_mila_ldap.query_ldap": 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 @@ -198,7 +247,7 @@ def test_acquire_users_supervisors(
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 @@ -221,8 +270,8 @@ def test_acquire_users_supervisors(

# 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 @@ -268,12 +317,16 @@ 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

patch_return_values(
{
"sarc.users.read_mila_ldap.query_ldap": 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 @@ -282,7 +335,7 @@ def test_acquire_users_co_supervisors(
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 @@ -305,8 +358,8 @@ def test_acquire_users_co_supervisors(

# 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 @@ -411,12 +464,26 @@ def test_acquire_users_prompt(

assert spans[1].name == "match_drac_to_mila_accounts"
assert spans[1].status.status_code == StatusCode.OK
assert len(spans[1].events) == 5
assert len(spans[1].events) == 10
assert (
spans[1].events[0].name
== "Loading mila_ldap, drac_roles and drac_members from files ..."
)
assert spans[1].events[1].name == "Loading matching config from file ..."
assert spans[1].events[2].name == "Matching DRAC/CC to mila accounts ..."
assert spans[1].events[3].name == "Committing matches to database ..."
assert spans[1].events[4].name == "Saving 1 manual matches ..."
assert spans[1].events[3].name == "Applying users delegation exceptions ..."
assert (
spans[1].events[4].name
== "Applying delegation exception for [email protected] ..."
)
assert spans[1].events[5].name == "Applying users supervisor exceptions ..."
assert (
spans[1].events[6].name
== "Applying supervisor exception for [email protected] ..."
)
assert (
spans[1].events[7].name
== "Applying supervisor exception for [email protected] ..."
)
assert spans[1].events[8].name == "Committing matches to database ..."
assert spans[1].events[9].name == "Saving 1 manual matches ..."
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Found user:
"active": true
},
"drac": null,
"teacher_delegations": null,
"mila_ldap": {
"co_supervisor": null,
"display_name": "M/Ms Bonhomme",
Expand Down
3 changes: 3 additions & 0 deletions tests/functional/client/test_func_get_user/test_get_users.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Found 3 users(s):
"active": true
},
"drac": null,
"teacher_delegations": null,
"mila_ldap": {
"co_supervisor": null,
"display_name": "M/Ms Bonhomme",
Expand Down Expand Up @@ -34,6 +35,7 @@ Found 3 users(s):
"email": "[email protected]",
"active": true
},
"teacher_delegations": null,
"mila_ldap": {
"co_supervisor": null,
"display_name": "M/Ms Petitbonhomme",
Expand Down Expand Up @@ -74,6 +76,7 @@ Found 3 users(s):
"email": "[email protected]",
"active": true
},
"teacher_delegations": null,
"mila_ldap": {
"co_supervisor": null,
"display_name": "M/Ms Beaubonhomme",
Expand Down
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

0 comments on commit f91b467

Please sign in to comment.