Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve logging on SISU interactions #1157

Closed
PasiSa opened this issue Feb 2, 2023 · 2 comments · Fixed by #1159
Closed

Improve logging on SISU interactions #1157

PasiSa opened this issue Feb 2, 2023 · 2 comments · Fixed by #1159
Assignees
@PasiSa
Labels
area: admin Issues related to server administration and service upkeep effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: team The issue is raised by a person inside the A+ developer team
Milestone
v1.18

Comments

@PasiSa
Copy link
Contributor

PasiSa commented Feb 2, 2023

  • SISU logs should show also the removed students when enrollments are updated
  • Logs should show which course failed, when query fails
@PasiSa PasiSa self-assigned this Feb 2, 2023
@PasiSa PasiSa moved this to Todo in A+ sprints Feb 2, 2023
@PasiSa PasiSa added area: admin Issues related to server administration and service upkeep effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: team The issue is raised by a person inside the A+ developer team labels Feb 2, 2023
@markkuriekkinen
Copy link
Contributor

SISU logs should show also the removed students when enrollments are updated

easy to add here?

a-plus/course/models.py

Lines 949 to 950 in 8b0d6e6

qs = Enrollment.objects.filter(user_profile__in=to_remove, course_instance=self)
qs.update(status=Enrollment.ENROLLMENT_STATUS.REMOVED)

Logs should show which course failed, when query fails

Is the query done in the Sisu plugin?
https://github.com/apluslms/aalto-sisu/blob/master/aalto_sisu/sis_aalto.py

PasiSa added a commit to PasiSa/a-plus that referenced this issue Feb 3, 2023
@PasiSa
Improve logging and output on SIS interactions
c1cc2b3 
Show also the number of removed students in log and UI notification.
If error occurs, show the course that was being fetched.

Closes apluslms#1157
@PasiSa PasiSa mentioned this issue Feb 3, 2023
Merged
18 tasks
@PasiSa
Copy link
Contributor Author

PasiSa commented Feb 3, 2023

I don't think we need to touch Sisu plugin after all.

@PasiSa PasiSa moved this from Todo to Under review in A+ sprints Feb 3, 2023
PasiSa added a commit to PasiSa/a-plus that referenced this issue Feb 3, 2023
@PasiSa
Improve logging and output on SIS interactions
8d529c4 
Show also the number of removed students in log and UI notification.
If error occurs, show the course that was being fetched.

Closes apluslms#1157
markkuriekkinen pushed a commit that referenced this issue Feb 3, 2023
@PasiSa @markkuriekkinen
Improve logging and output on SIS interactions
848cf9a 
Show also the number of removed students in log and UI notification.
If error occurs, show the course that was being fetched.

Closes #1157
@github-project-automation github-project-automation bot moved this from Under review to Done in A+ sprints Feb 3, 2023
@markkuriekkinen markkuriekkinen added this to the v1.18 milestone Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@PasiSa PasiSa

Labels
area: admin Issues related to server administration and service upkeep effort: hours Estimated to take less than one day, from the creation of a new branch to the merging experience: beginner required knowledge estimate requester: team The issue is raised by a person inside the A+ developer team
Projects
A+ sprints
Status: Done
Milestone
v1.18
Development

Successfully merging a pull request may close this issue.

Improve logging and output on SIS interactions PasiSa/a-plus
2 participants
@PasiSa @markkuriekkinen