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.

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

Add Forum v2 request logging #571

Merged
merged 2 commits into from
Aug 2, 2024
Merged

Conversation

taimoor-ahmed-1
Copy link

@taimoor-ahmed-1 taimoor-ahmed-1 commented Jul 29, 2024

@taimoor-ahmed-1 taimoor-ahmed-1 self-assigned this Jul 29, 2024
@taimoor-ahmed-1 taimoor-ahmed-1 force-pushed the feat/add_forum_v2_request_logging branch 2 times, most recently from a268a18 to 1a95b90 Compare July 29, 2024 12:29
send request to forum v2 alongside v1
validate both responses and log info in case of diff
@taimoor-ahmed-1 taimoor-ahmed-1 force-pushed the feat/add_forum_v2_request_logging branch from 1a95b90 to 1fff44e Compare July 30, 2024 06:12
@taimoor-ahmed-1 taimoor-ahmed-1 marked this pull request as draft July 30, 2024 13:19
- compare json objects instead of response objects
- format the added lines of code
@Faraz32123 Faraz32123 force-pushed the feat/add_forum_v2_request_logging branch from 7581470 to afe87f8 Compare August 1, 2024 14:40
Copy link

@Faraz32123 Faraz32123 left a comment

Choose a reason for hiding this comment

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

LGTM!
I have tested it, It worked well!

@Faraz32123 Faraz32123 marked this pull request as ready for review August 1, 2024 14:48
@Faraz32123
Copy link

One Unit test is failing due to our change in code as it tries to access our url, and it fails.

2024-08-01T15:08:00.4392693Z lms/djangoapps/discussion/rest_api/tests/test_api.py:342: in get_course_topics
2024-08-01T15:08:00.4392993Z     return get_course_topics(self.request, self.course.id)
2024-08-01T15:08:00.4393414Z lms/djangoapps/discussion/rest_api/api.py:526: in get_course_topics
2024-08-01T15:08:00.4393723Z     thread_counts = get_course_commentable_counts(course.id)
2024-08-01T15:08:00.4394483Z openedx/core/djangoapps/django_comment_common/comment_client/course.py:33: in get_course_commentable_counts
2024-08-01T15:08:00.4394666Z     response = perform_request(
2024-08-01T15:08:00.4395255Z openedx/core/djangoapps/django_comment_common/comment_client/utils.py:80: in perform_request
2024-08-01T15:08:00.4395377Z     response_v2 = requests.request(

merging this PR for as we might change the code again.

@Faraz32123 Faraz32123 merged commit f7be991 into forum_v2 Aug 2, 2024
45 of 48 checks passed
@Faraz32123 Faraz32123 deleted the feat/add_forum_v2_request_logging branch August 2, 2024 06:17
Faraz32123 pushed a commit that referenced this pull request Sep 16, 2024
* feat: Add Forum v2 request logging
send request to forum v2 alongside v1
validate both responses and log info in case of diff

* feat: modify comparison and formatting
- compare json objects instead of response objects
- format the added lines of code

---------

Co-authored-by: Taimoor  Ahmed <[email protected]>
Co-authored-by: Muhammad Faraz  Maqsood <[email protected]>
Faraz32123 pushed a commit that referenced this pull request Sep 16, 2024
* feat: Add Forum v2 request logging
send request to forum v2 alongside v1
validate both responses and log info in case of diff

* feat: modify comparison and formatting
- compare json objects instead of response objects
- format the added lines of code

---------

Co-authored-by: Taimoor  Ahmed <[email protected]>
Co-authored-by: Muhammad Faraz  Maqsood <[email protected]>
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