All the changes that have been made in the last meeting#4
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the backend’s Celery messaging infrastructure from Redis to RabbitMQ and updates related configuration/docs, while also introducing several runtime/debug changes in the attendance flow and adding a basic health check endpoint.
Changes:
- Switched Celery broker to RabbitMQ and changed result backend to
rpc://; replaced Redis cache with DjangoLocMemCache. - Updated documentation to reflect RabbitMQ setup and added a RabbitMQ migration summary doc.
- Modified attendance processing endpoints/tasks (debug output, input validation changes, and URL generation inputs) and added a
/health/endpoint; updated CORS origins.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates setup docs from Redis to RabbitMQ; adds RabbitMQ run instructions and env var. |
| RABBITMQ_MIGRATION.md | New migration summary documenting the Redis→RabbitMQ changes. |
| ClassLens_DB/start_server.bat | Adds a Windows batch script to start the server via Waitress. |
| ClassLens_DB/Home/views.py | Adjusts student password/embedding handling, attendance start validation/logging, hard-coded host for task, and adds health view. |
| ClassLens_DB/Home/urls.py | Minor formatting-only change. |
| ClassLens_DB/Home/tasks.py | Adds a debug print in evaluate_attendance. |
| ClassLens_DB/DatabaseAdminApp/settings.py | Adds a Vercel frontend domain to allowed CORS origins. |
| ClassLens_DB/ClassLens_DB/urls.py | Adds /health/ route. |
| ClassLens_DB/ClassLens_DB/settings.py | Replaces Redis cache and Celery broker settings with RabbitMQ/locmem/rpc backend. |
| ClassLens_DB/ClassLens_DB/celery.py | Updates Celery app configuration to use RABBITMQ_URL and rpc://. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| docker run -d --name classlens-rabbitmq -p 5672:5672 -p 15672:15672 rabbitmq:3-management | ||
| ``` | ||
|
|
||
| //// |
| cd C:\Users\Admin\Desktop\ClassLens\ClassLens_DB | ||
| call C:\Users\Admin\Desktop\ClassLens\venv\Scripts\activate.bat |
There was a problem hiding this comment.
no need to add this bat file. Add it in gitignore
| embedding=registerNewStudent(request.FILES.get("photo")) | ||
| if not isinstance(embedding,Exception): | ||
| student.face_embedding = embedding | ||
| else : | ||
| raise Exception("Face embedding error: " + str(embedding)) | ||
| except Exception as e : | ||
| return Response({"error": "Face Not Detected, Upload A New Image"}, status=status.HTTP_400_BAD_REQUEST) |
| # Debug log to see exactly what is received | ||
| print("=" * 60) | ||
| print("MARK ATTENDANCE REQUEST RECEIVED") | ||
| print(f" photos : {photos}") | ||
| print(f" subject_id : {subject_id!r}") | ||
| print(f" teacher_id : {teacher_id!r}") | ||
| print(f" department : {departmentName!r}") | ||
| print(f" year : {year!r}") | ||
| print("=" * 60) |
| if missing: | ||
| return Response({"error": f"Missing required fields: {', '.join(missing)}"}, status=400) | ||
|
|
||
| if int(teacher_id) == 0: |
| ) | ||
|
|
||
| task = evaluate_attendance.delay(total_sessions,class_session.id,request.scheme, request.get_host()) | ||
| task = evaluate_attendance.delay(total_sessions,class_session.id,request.scheme,"14.139.121.110:11020") |
| session.subject.name, | ||
| session.class_datetime | ||
| ) | ||
| print(f"images url {image_urls[0]}") |
| "BACKEND": "django.core.cache.backends.locmem.LocMemCache", | ||
| "LOCATION": "classlens-local-cache", |
There was a problem hiding this comment.
@mannshah24 LocMemCache is not suitable for multi-worker (like ours ) production deployments because each process maintains its own isolated in-memory cache.
The suggestion by copilot seems appropriate to use db as a cache.
First run this command python manage.py createcachetable classlens_cache and then apply the suggested changes by copilot.
| CELERY_BROKER_URL = env('REDIS_URL') | ||
| CELERY_RESULT_BACKEND = env('REDIS_URL') | ||
| CELERY_BROKER_URL = env('RABBITMQ_URL') | ||
| CELERY_RESULT_BACKEND = 'rpc://' |
There was a problem hiding this comment.
@mannshah24
Since using rabbitMQ will result in some inconsistencies like :
1.) Using rpc:// as the celery_result_backend associates the queue with the process (worker) so while polling if the response is received by a different worker then the one who submitted the job to celery then it won't be able to find the task_id and so the app will keep on polling until the response is received by the same worker who assigned the job to celery
2.) The rpc:// can be avoided by using amqp:// but it causes memory overflow and it is not reliable as well if the network connection somehow breaks b/w the app and our server
So the reliable and feasible way is to use django-db
Just add 'django_celery_results' in the installed app list and
CELERY_RESULT_BACKEND = 'django-db'
in settings.py
and run the command :
pip install django-celery-results
python manage.py migrate django_celery_results
That's it no other changes are required, just make this changes
| year = models.IntegerField() | ||
| semester = models.IntegerField() | ||
| division = models.CharField(max_length=20) # A, B, C, etc. | ||
| raw_payload = models.JSONField(default=dict) |
There was a problem hiding this comment.
What does raw_payload contains, for all the tables defined what does raw_payload contains ?
There was a problem hiding this comment.
What is the use of APIPaper, APIStudentAcademicInformation and APIStudentPartTerm.... ? What are the uses of this tables and it's data for our system ?
There was a problem hiding this comment.
Add the vercel url in the env and then access it from there, don't hardcore the url in the code
There was a problem hiding this comment.
What is apply_to_core in the sync method ?
| class Subject(models.Model): | ||
| code = models.TextField(unique=True, null=False,default="") | ||
| name = models.TextField(null=False) | ||
| department = models.ForeignKey(Department, on_delete=models.SET_NULL, null=True, blank=True) |
There was a problem hiding this comment.
Why is department added in the subject model ? That's why we had SubjectFromDept table why is department added in the subject table ? Subject table was made to just store the raw subject being taught in the university why is the department field added ? It creates insertion anomaly and is not even in 2nd Normal Form
| }) | ||
|
|
||
| # compute overall attendance across all subjects (weighted by total sessions) | ||
| total_classes_sum = sum(item.get('total', 0) for item in subjects_data) |
There was a problem hiding this comment.
No need for recalculating the total_classes_sum and the attended_sum, we are storing it in the StudentAttendancePercentage, from where we can get the attendance of the student and from ClassSession Table we can get the total count of lectures as well no need for calculating the variables again and again
VyomShah28
left a comment
There was a problem hiding this comment.
Added some comments regarding changes that need to be made
…to dhriti-backend
Dhriti backend
resolve the timestamp mismatch between class_session_time and attenda…
No description provided.