Skip to content

Remove log_task_exceptions_to_django_logger#328

Merged
cc-a merged 3 commits intomainfrom
293_task_log
Mar 5, 2026
Merged

Remove log_task_exceptions_to_django_logger#328
cc-a merged 3 commits intomainfrom
293_task_log

Conversation

@SaranjeetKaur
Copy link
Contributor

Description

Please include a summary of the change and which issue is fixed (if any). Please also
include relevant motivation and context. List any dependencies that are required for
this change.

Fixes #314

Type of change

  • Documentation (non-breaking change that adds or improves the documentation)
  • New feature (non-breaking change which adds functionality)
  • Optimization (non-breaking, back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (whatever its nature)

Key checklist

  • All tests pass (eg. python -m pytest)
  • The documentation builds and looks OK (eg. python -m sphinx -b html docs docs/build)
  • Pre-commit hooks run successfully (eg. pre-commit run --all-files)

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added or an issue has been opened to tackle that in the future. (Indicate issue here: # (issue))

Copilot AI review requested due to automatic review settings March 4, 2026 10:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the log_task_exceptions_to_django_logger wrapper from imperial_coldfront_plugin.tasks, aligning task error-reporting with the updated approach described in issue #314 (emailing admins via the django-q logger rather than duplicating exceptions onto the django logger).

Changes:

  • Deleted the log_task_exceptions_to_django_logger decorator class and its wrapper assignments.
  • Promoted the previously-underscored task implementations (e.g., _check_ldap_consistency) to the public task entrypoints (e.g., check_ldap_consistency) by defining them directly under the exported names.
  • Removed now-unused imports (functools, Callable).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@SaranjeetKaur
Copy link
Contributor Author

SaranjeetKaur commented Mar 4, 2026

Hi @cc-a,

Could you please check the steps below? I am not sure if I am following the correct steps to update files in both, this submodule repo and the parent coldfront repo.

This is what I have done so far (and it creates 2 PRs, one in each of the repos):

  1. Created a new branch 293_task_log in imperial_coldfront_plugin repo and made changes to the tasks.py file
  2. Pushed these changes and created a PR on this repo
  3. Then did cd .. to go back to the coldfront repo and created a new branch there with the same name 293_task_log
  4. Changed the local_settings.py file in the coldfront repo
  5. Did cd imperial_coldfront_plugin and then did git pull origin 293_task_log
  6. Next went back to coldfront repo cd ..
  7. Staged both imperial_coldfront_plugin and local_settings.py in the coldfront repo. Then did a git commit and push to create a new PR in the coldfront repo

@cc-a
Copy link
Collaborator

cc-a commented Mar 4, 2026

Thanks @SaranjeetKaur. That's fine. We'll address this PR first then update the PR in the development environment repository to point to the merge commit created in main for the plugin before merging that.

@SaranjeetKaur
Copy link
Contributor Author

Ok thanks @cc-a. Will open this PR for your review then.

@SaranjeetKaur SaranjeetKaur requested review from Sahil590 and cc-a March 4, 2026 15:27
@cc-a cc-a merged commit ac16bcd into main Mar 5, 2026
4 checks passed
@cc-a cc-a deleted the 293_task_log branch March 5, 2026 10:11
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.

Task logging

3 participants