Skip to content

Conversation

@bradenmacdonald
Copy link
Contributor

Description: Two XBlocks specified in edx-private.txt both contain Django apps that try to create a table called mentoring_answers. This causes South to abort its migrations in some cases, due to the table already existing. Full details are in open-craft/problem-builder#18

This fix renames the table used by Problem Builder, so that there is no longer any conflict. It includes a data migration in case any important student data was saved to the old table.

Problem reported by @maxrothman.

JIRA: OSPR-547

Discussions: Fix discussed with @nedbat on HipChat 'dev' channel.

Internal Review: Not done yet

Merge deadline: ASAP since it blocks DB sync on fresh installs.

Partner information: hosted on edx.org (Harvard)

Sandbox: I tested the DB update when neither table existed using http://sandbox2.opencraft.com:18010/

Note: This should not be merged until open-craft/problem-builder#18 is merged, and the hash updated.

@openedx-webhooks
Copy link

Thanks for the pull request, @bradenmacdonald! I've created OSPR-547 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

  • supporting documentation
  • edx-code email threads
  • timeline information ('this must be merged by XX date', and why that is)
  • partner information ('this is a course on edx.org')
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the Github pull request interface. As a reminder, our process documentation is here.

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U needs triage labels Apr 23, 2015
@sarina
Copy link
Contributor

sarina commented Apr 23, 2015

@bradenmacdonald how can we tell if there's any students using the block yet?

@bradenmacdonald
Copy link
Contributor Author

@sarina That's tricky since the mentoring_answers table would hold answers from all previous courses as well as students potentially using the new block in a new course. Since it's a brand new block the easiest way would be to check with the course authors who were about to use it... If there's a specific course that only uses the new Problem Builder block that we're worried about, we can verify with:

$ ./manage.py lms shell --settings=devstack
>>> from mentoring.models import Answer
>>> Answer.objects.filter(course_id="HarvardX/GSE1.1x/3T2014").count()
0

@sarina
Copy link
Contributor

sarina commented Apr 23, 2015

it seems like it'd be safest to include the data migration, no? @maxrothman thoughts?

@maxrothman
Copy link
Contributor

@sarina definitely agree.

@bradenmacdonald
Copy link
Contributor Author

@sarina Yeah, good point. I'll add it in.

@bradenmacdonald bradenmacdonald force-pushed the fix-mentoring-answers-conflict branch from 44ec4b5 to b996ce0 Compare April 23, 2015 22:49
@antoviaque
Copy link
Contributor

There shouldn't be any real student answer in problem-builder yet, since the course hasn't started - it would simply have emptied any test instructors would have made. But it doesn't hurt to have the migration yes.

LGTM - I also tested on a fresh sandbox that didn't include mentoring or problem-builder, the migrations passed correctly. 👍

@sarina
Copy link
Contributor

sarina commented Apr 24, 2015

@bradenmacdonald 👍

Do you need to merge open-craft/problem-builder#18 and then update the hash here before this merges?

@openedx-webhooks openedx-webhooks added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed community manager review labels Apr 24, 2015
@bradenmacdonald bradenmacdonald force-pushed the fix-mentoring-answers-conflict branch from b996ce0 to 8dc4ecb Compare April 24, 2015 16:00
@bradenmacdonald
Copy link
Contributor Author

@sarina Yep, I've just merged the other PR and updated this hash now. Will merge once the build passes.

bradenmacdonald added a commit that referenced this pull request Apr 24, 2015
Fix for DB table conflict preventing DB update on fresh installs
@bradenmacdonald bradenmacdonald merged commit 81a5822 into openedx:master Apr 24, 2015
@bradenmacdonald bradenmacdonald deleted the fix-mentoring-answers-conflict branch April 24, 2015 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants