Bump problem-builder to v2.6.5

[mtyaka]

This patch fixes the problem where Problem Builder Long Answer blocks throw an error when used inside courses with long course keys (> 50 characters), see TNL-5932.

It introduces a new 255-char course_key field to the Answer model. The old 50-char course_id field is still present and used for fallback, but is deprecated and will be removed in next release.

This release replaces the initial approach from https://github.com/edx/edx-platform/pull/14013, because it was found that simply extending the existing course_id column isn't going to work well in production because the table affected by the migration is quite large and extending column length requires locking the table for the duration of the migration.

For more information on the new approach proposed by @maxrothman in https://github.com/edx/edx-platform/pull/14013#issuecomment-261635454, see open-craft/problem-builder#131.

Below is the output of sqlmigrate for the new migration:

BEGIN;
ALTER TABLE `problem_builder_answer` ADD COLUMN `course_key` varchar(255) NULL;
ALTER TABLE `problem_builder_answer` ADD CONSTRAINT `problem_builder_answer_student_id_2ce682a818c95cbc_uniq` UNIQUE (`student_id`, `course_key`, `name`);
CREATE INDEX `problem_builder_answer_c8235886` ON `problem_builder_answer` (`course_key`);

COMMIT;

Next release will add a data migration to copy contents of course_id to the new course_key column. EdX will fake this migration and copy data in batches instead.
Next release will also drop the deprecated course_id column.

See https://github.com/edx/edx-platform/pull/14013#issuecomment-261635454 for more information.

Environments: edx.org and edge.edx.org

Merge deadline: ASAP - this is blocking a course on Edge

JIRA tickets: TNL-5932

Discussions: https://github.com/edx/edx-platform/pull/14013, open-craft/problem-builder#131 and TNL-5932

Sandbox URL:

Sandboxes contain revision 86177cb.

There is a course with course key longer than 50 characters that already contains some long answer blocks.

I also created a course that was prepared with an older version of problem builder using now deprecated course_id field.

• LMS: https://pr14044.sandbox.opencraft.hosting/
• Studio: https://studio-pr14044.sandbox.opencraft.hosting/

Partner information: hosted on edX Edge (Davidson College)

Testing instructions:

Check that you can successfully add a Problem Builder "Long Answer" component to a course with a course key that is longer than 50 character and that answers answered on an older version of problem-builder continue to work correctly with this patch.

1. Install problem-builder v2.6.1
2. Create a course with a combined course key less than 50 characters.
3. Add a Problem Builder component, then add the Long Answer component.
4. Answered several questions.
5. Create a course with a combined course key more than 50 characters.
6. Add Unit with Problem Builder and a Long Answer element - watched it fail.
7. Installed problem-builder from this PR.
8. Run migrations.
9. Verify that the answers are still present in the first course.
10. Verify that the Long Answer element in the course with the very long course ID is working now, and you can submit answers to it.

Reviewers

• @haikuginger
• @adampalay
• @maxrothman / @edx/devops

Settings

NGINX_SET_X_FORWARDED_HEADERS: false
INSIGHTS_VERSION: '58086e85d673e692cd498793905d8739cfc01515'
EDXAPP_EXTRA_REQUIREMENTS:
  - name: git+https://github.com/open-craft/problem-builder.git@v2.6.5#egg=xblock-problem-builder==2.6.5

[haikuginger]

👍

• I tested this:
  • I added a Long Answer to a course with a short key, and it worked
  • I submitted an answer to that Long Answer element, and it was saved
  • I created a course with a long key, and attempted to add a Long Answer to it
  • I verified that the added Long Answer failed to render
  • I installed 2.6.4 and ran migrations.
  • I verified that the Long Answer in the course with the long key rendered
  • I verified that I was able to submit answers to the course with the long key
  • I verified that existing answers for the course with the short key were still present.
• I read through the code
• I checked for accessibility issues N/A
• Includes documentation N/A

[adampalay]

@mtyaka , why are we changing the name of course_id to course_key? That might have further down ramifications for people making reports off that table. Or was that something you wanted to do eventualy anyway?

[mtyaka]

@adampalay We need two columns during the transition period. We can rename course_key to course_id after all data has been copied over and we drop the old course_id column.

[adampalay]

@mtyaka right, my thought was if we wanted to make a duplicate column, should we be more explicit about its name? Should we call it course_id_transition?

It's a nit, but that was my question.

[mtyaka]

@adampalay Ah, I see. I did not plan to rename the column back to course_id originally, which is why I went with course_key, but I did not think anybody would be using the column directly.

I we decide to rename the column back to course_id after the transition is done, course_id_transition would indeed make more sense.

[maxrothman]

I'm not entirely sure whether renaming a column in mysql results in a full column copy. If it does, we'd have to make the name of the new column semantic. Otherwise, I'd vote to rename it. You should be able to test which it is by making a big table, issuing the ALTER TABLE command, and seeing how long it takes.

[maxrothman]

@mtyaka can you link to the relevant changes in the problem builder repo?

[bradenmacdonald]

@maxrothman Do you mean this: open-craft/problem-builder#131 ?

[maxrothman]

Yep, thank you.

[mtyaka]

@maxrothman I tried renaming a column with LOCK=NONE and it did not complain, so it looks like renaming a column is quick (this is on MySQL 5.6.24):

ALTER TABLE `problem_builder_answer` CHANGE `course_id_transition` `course_id` varchar(50) NOT NULL, LOCK=NONE;
Query OK, 0 rows affected (0.00 sec)
Records: 0  Duplicates: 0  Warnings: 0

Thinking about it, renaming a column that is in use seems be a bit hard from operations perspective. At the moment that you run the migration, old code that is using the old column name stops working, so you would have to switch all servers to the new release immediately after running the migration or you will see errors in production. Is that a concern? Is there a common pattern for situations like this?

[maxrothman]

Yeah, I think renaming the column in-place is infeasible. If we want the column to have the same name in the end, we'll have to repeat the whole process a second time, copying back to an identical column with the original name.

[mtyaka]

Thanks for the confirmation @maxrothman.

@adampalay Do you think it's worth the trouble of migrating the data twice to be able to end up with the same name (column_id) in the end or is it acceptable if the final column name is different (course_key)?

[mtyaka]

@adampalay How would you like to proceed?

[bradenmacdonald]

@maxrothman @adampalay @mtyaka This issue is CAT-2 and is preventing some students from accessing some courses - we need to find a way forward ASAP.

[adampalay]

@mtyaka , my instinct is that it's not worth the trouble to keep the name. But we'd have to spread that information out to folks.

Sorry for the delay; I was an an offsite that week and fell behind on emails.

[adampalay]

@maxrothman , may you please review?

[maxrothman]

Whoops! I started a review on open-craft/problem-builder#131 but never submitted it.

[mtyaka]

Thanks @maxrothman! I opened open-craft/problem-builder#136 to address your comment on open-craft/problem-builder#131. Can you please review?

[mtyaka]

@maxrothman I bumped the tag to v2.6.5 to include the changes from open-craft/problem-builder#136. The sandbox has been updated.

Can you please approve this version bump if it looks good?

[mtyaka]

Thank you.

[bradenmacdonald]

@mtyaka @adampalay @maxrothman I'm really glad to see this merged, and I know Davidson will be too. Can we do a hotfix for this? See request on TNL-5932.

[adampalay]

@bradenmacdonald we can probably cherry-pick this onto the RC. @sanfordstudent what's the best way for us to add this commit to the RC? Can you cherry-pick it directly, or should @mtyaka make a PR against release-candidate?

[sanfordstudent]

A PR against release-candidate would be best. Will this change require any migrations?

Thanks!

@adampalay @mtyaka @bradenmacdonald

[adampalay]

@sanfordstudent , I'm afraid it would. @mtyaka , may you please make a new PR against release-candidate?

[mtyaka]

PR against release-candidate opened at https://github.com/edx/edx-platform/pull/14143

@adampalay @sanfordstudent

[sanfordstudent]

Sorry @mtyaka - after talking to devops we'd like this to go in as a hotfix instead of an update to the release candidate.

[mtyaka]

@sanfordstudent No problem. Is there anything left for me to do or is it in your/edX hands now?

[sanfordstudent]

@mtyaka @adampalay the hotfix process is documented here: https://openedx.atlassian.net/wiki/pages/viewpage.action?pageId=25559520

I don't believe that I am supposed to be the one to shepherd the hotfix through release, but please let me know if I'm wrong and I'll be happy to.

[adampalay]

@mtyaka , edx has it from here. @sanfordstudent , I'll check in with you offline about ushering it through!

[cpennington]

This PR is being deployed to edx.org Staging in preparation for a release to production on Monday, 12/19 at 10 am. If you feel you need to validate it manually, please do so by then, and contact me if there is an urgent enough issue to scrap the deployment.

[cpennington]

This PR is being deployed to edx.org Staging in preparation for a release to production on Tuesday, 12/20 at 10 am. If you feel you need to validate it manually, please do so by then, and contact me if there is an urgent enough issue to scrap the deployment.

[cpennington]

This PR has been deployed to edx.org Staging in preparation for a release to production on Wednesday, 12/21/16 at 10 am. If you feel you need to validate it manually, please do so by then, and contact me if there is an urgent enough issue to scrap the deployment.