Skip to content

Conversation

@nurbal
Copy link
Collaborator

@nurbal nurbal commented Feb 23, 2024

This PR adresses two tickets:
SARC-292 : add delegates informations (one user can see another prof's students infos)
SARC-293 : override supervisor/co_supervisor

Both tickets are based on exceptions listed in secrets/exceptions.json
The mock of this file is implemented in conftest.py

@nurbal nurbal marked this pull request as draft February 23, 2024 06:01
@nurbal nurbal marked this pull request as ready for review March 1, 2024 06:38
@nurbal nurbal requested a review from bouthilx March 1, 2024 06:40
# "drac_members": {...} or None
# }

exceptions = load_ldap_exceptions(cfg.ldap)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On essaie de partitionner chaque phase de chargement pour pouvoir les convertir en module avec une interface générique. Par exemple, le code de la ligne 35 https://github.com/mila-iqia/SARC/pull/103/files#diff-e9c775d5112b754dc796c041cfd42e5feddc9419366751fe3e0ffa2b9a025a73R35 aurait du se retoruver dans fetch_mymila.

Ces exceptions sont surtout en lien avec mymila, je crois que ça devrait aussi être dans fetch_mymila.

Copy link
Collaborator Author

@nurbal nurbal Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

En l'occurrence, pas tout à fait. Pour les exceptions de superviseurs, OUI c'est en lien avec MyMila. Par contre pour les délégations de supervision, NON cette info n'est pas présente dans les 3 sources de données (DRAC, LDAP et MyMila) donc elles ont un scope "général".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ce que je veux dire, c'est que ces exceptions sont appliquées après le matching des 3 sources de données, indépendemment de la provenance des valeurs des champs concernés. Elles sont "agnostiques" si on veut...
Ça me paraissait donc logique de les appliquer à ce niveau là.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, donc dans un soucis de rendre ça le plus générique possible, ça serait comme un module supplémentaire dans le pipeline qui ajoute des corrections aux sources précédentes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fxed in 9249978

@nurbal nurbal merged commit ee50c58 into master Oct 1, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants