mitodl / ccxcon

CCXCon API
GNU Affero General Public License v3.0
7 stars 0 forks source link

Support for syncing modules. #89

Closed justinabrahms closed 8 years ago

justinabrahms commented 8 years ago

What's this PR do?

Adds support for fetching course structure (aka Modules) from the edx API.

Where should the reviewer start?

courses/tasks.py

How should this be manually tested?

  1. Create an oauth application in your edx devstack. You should create a backing user for the application as well. Type should be Confidential.
  2. Create a grant with the same backing user.
  3. Take the tokens and username generated in 1 and 2 and add it to ccxcon. The access token, refresh token, and expiration date should all stay as their default values (empty for tokens, 1970 for expiration).
  4. Add user you created as a staff member of the course in question.
  5. Load it up in ./manage.py shell and trigger a save on the course you want to trigger an update for. Ensure it has a valid course_id.
  6. course.module_set.all() should return a list of that course's modules in order.

    Any background context you want to provide?

I've moved edx_instance to it's own fully-formed model so we can store additional OAuth information. This is in a new app called oauth_mgmt.

You can now exclude slow tests with -m \"not slowtest\" which will skip dredd from running.

What are the relevant tickets?

Fixes #91 #9 #5

What gif best describes this PR or how it makes you feel?

pdpinch commented 8 years ago

Fixes #91?

noisecapella commented 8 years ago

You don't need to change the description here but some notes for future documentation:

I have successfully run through the manual steps, onto the code review

noisecapella commented 8 years ago

Is Celery delegating out tasks to workers? I'm seeing everything run in the web container, not the celery container (cc @bdero )

noisecapella commented 8 years ago

Migrations work fine

noisecapella commented 8 years ago

This is out of the scope for this PR but we should consider disabling PATCH/PUT/DELETE on courses and also POST for modules, if no client has a reasonable use for them, so we don't have to worry about handling those cases. (#97)

noisecapella commented 8 years ago

When I POST an incorrect course id to courses I get an immediate retry failure instead of the exponential backoff. I think this is related to celery tasks running in the web process instead of in their own workers

noisecapella commented 8 years ago

Overall this looks good. I feel like the Celery issue is a configuration problem, any insight @bdero?

justinabrahms commented 8 years ago

I agree that this is a configuration error. We're currently doing eager celery running, which means that it executes in-process as a blocking request. If we swap up CELERY_ALWAYS_EAGER to False, it'll use the celery container.

noisecapella commented 8 years ago

We should change it to be False except in tox.ini because only tests should need the eager functionality. I'll file a ticket for that

noisecapella commented 8 years ago

Looks good to me (remember to squash!) :+1: