Skip to content
This repository has been archived by the owner on Feb 27, 2024. It is now read-only.

Conversation

Agrendalath
Copy link
Contributor

@Agrendalath Agrendalath commented Oct 7, 2018

This PR optimizes listing and removing users in organizations.

Testing instructions (for solutions devstack):

Add a test organization and some users:

  • add a test organization:
curl -X POST \
  http://localhost:8000/api/server/organizations/ \
  -H 'Cache-Control: no-cache' \
  -H 'Content-Type: application/json' \
  -H 'Postman-Token: b38bc64c-f147-493e-a0e1-8fd6df8b147b' \
  -H 'X-Edx-Api-Key: PUT_YOUR_API_KEY_HERE' \
  -H 'content-type: multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW' \
  -F name=TestOrganization
  • add some users to the organization (the best would be to have more than 1 user there):
curl -X POST \
  http://localhost:8000/api/server/organizations/1/users/ \
  -H 'Cache-Control: no-cache' \
  -H 'Content-Type: application/json' \
  -H 'Postman-Token: 6ca75b56-1521-442c-80fa-382f313c6f95' \
  -H 'X-Edx-Api-Key: PUT_YOUR_API_KEY_HERE' \
  -H 'content-type: multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW' \
  -F id=5

(change id of user in the last line)

Install silk:

  • run pip install django-silk==2.0.0 (3.0.0 dropped Django<1.11.0 support),
  • add the following to common.py:
MIDDLEWARE = [
    ...
    'silk.middleware.SilkyMiddleware',
    ...
]

INSTALLED_APPS = (
    ...
    'silk'
)
  • add urlpatterns += [url(r'^silk/', include('silk.urls', namespace='silk'))] to urls.py,
  • run ./manage.py lms migrate silk --settings=devstack,

Make requests:

  • make a couple (single requests are pretty inaccurate) of GET and DELETE requests on the edx-solutions:master branch
    GET (adjust course_id in URL param):
curl -X GET \
  'http://localhost:8000/api/server/organizations/1/users/?include_course_counts=true&include_grades=true&course_id=123/123/2018_T4' \
  -H 'Cache-Control: no-cache' \
  -H 'Postman-Token: ab6f2fde-9798-4215-939b-b3361fda9691' \
  -H 'X-Edx-Api-Key: PUT_YOUR_API_KEY_HERE'

DELETE (adjust user ids in last line):

curl -X DELETE \
  http://localhost:8000/api/server/organizations/1/users/ \
  -H 'Cache-Control: no-cache' \
  -H 'Content-Type: application/json' \
  -H 'Postman-Token: f60a5b25-d0d6-4f76-974f-3f649f184f62' \
  -H 'X-Edx-Api-Key: PUT_YOUR_API_KEY_HERE' \
  -H 'content-type: multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW' \
  -F 'users=9,5,1'
  • checkout into branch of this PR and also make a couple of the GET and DELETE requests from above

Check the results:

  • access localhost:8000/silk and see difference in number of queries and time on edx-solutions:master and open-craft:agrendalath/MCKIN-8417-optimize-OrganizationsViewSet.users-API; you can also analyze the SQL tab under each request

Copy link
Contributor

@UmanShahzad UmanShahzad left a comment

Choose a reason for hiding this comment

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

👍

  • I tested this: works as expected from test instructions.
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

@UmanShahzad UmanShahzad merged commit 6010f2e into edx:master Oct 9, 2018
@UmanShahzad UmanShahzad changed the title [MCKIN-8417] WIP: Optimize OrganizationsViewSet.users API [MCKIN-8417] Optimize OrganizationsViewSet.users API Oct 9, 2018
@Agrendalath Agrendalath deleted the agrendalath/MCKIN-8417-optimize-OrganizationsViewSet.users-API branch October 9, 2018 13:21
@edx edx deleted a comment from openedx-webhooks Oct 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants