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

Added perseus -> assessment item conversion to restore_channel command #1482

Merged
merged 4 commits into from Jul 8, 2019

Conversation

jayoshih
Copy link
Contributor

@jayoshih jayoshih commented Jul 2, 2019

Description

Downloads the perseus zip from the download server and reads it to create perseus exercises

Steps to Test

  • Run restore_channel command on a channel with exercises

Checklist

  • Is the code clean and well-commented?
  • Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
  • Are there tests for this change?

@jayoshih jayoshih requested a review from kollivier July 2, 2019 23:52
@rtibbles
Copy link
Member

rtibbles commented Jul 2, 2019

Maybe I am missing something - but why does the restore_channel command use the published files and database, rather than just directly gathering the data from Studio APIs - wouldn't that save us a lot of this conversion work?

@jayoshih
Copy link
Contributor Author

jayoshih commented Jul 2, 2019

@rtibbles The reason for this is that we might be pulling from different servers, so some of the data might not be available when we run this command

@rtibbles
Copy link
Member

rtibbles commented Jul 3, 2019

Feel like I am still missing something, which different servers? If Studio has the published channel, why wouldn't it have the underlying data?

@kollivier
Copy link
Contributor

@rtibbles Also, a big part of the reason I asked for this is that it will enable offline sharing. Even if you don't have access to Studio online, as long as you can point this command to a file location that has a channel db and file structure that matches the 'official' Studio one, you can import the content into Studio and start curating. (Think those torrented channels we were talking about. ;-)

@jayoshih Is there a way we can get some tests for this using a couple perseus examples? Especially since we're doing format conversions for the assessment items, it'd be good if we can make sure they work as expected.

@codecov
Copy link

codecov bot commented Jul 3, 2019

Codecov Report

Merging #1482 into develop will decrease coverage by 0.33%.
The diff coverage is 13.69%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1482      +/-   ##
===========================================
- Coverage       79%   78.67%   -0.34%     
===========================================
  Files          246      246              
  Lines        11252    11313      +61     
===========================================
+ Hits          8890     8900      +10     
- Misses        2362     2413      +51
Impacted Files Coverage Δ
...tentcuration/contentcuration/utils/import_tools.py 26.24% <13.69%> (-3.76%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce24036...b92cca5. Read the comment docs.

@rtibbles
Copy link
Member

rtibbles commented Jul 3, 2019

The conversion from Perseus to the internal representation work will certainly be a useful tool when we start to think about deprecating the QTI supported subset of Perseus style exercises in favour of QTI.

I want to express a fairly strong concern about a concerted push to taking the entire Studio platform offline - this is something we had previously considered as the mechanism for enabling content curation in offline circumstances - but user research strongly suggested that the curriculum level alignment that Studio is designed to support is not what the majority of offline use cases require.

There are also several key technical reasons that offline distributed Studio would be problematic -- e.g. around preventing malicious ID collisions, central channel versioning, the email-based permissions and account systems, mechanisms for P2P synchronization, and the very online-focused and backend-intensive nature of many of Studio's features. As we begin to add more tools within Kolibri for offline integration of new content, the Studio feature set will also evolve towards centralized functionality for things like reviewing content that syncs back up, for alignment into existing channels, etc.

I understand that the work on it until now has been a side project, and that seems fine to me - but it's the slightly stronger impetus in this direction that I am sensing that I wanted to flag. We want to continue to build Studio to become the stable, scalable, central aggregation point for any offline content creation activities, and rearchitecting it to best support the distributed offline case would detract from this core purpose, and duplicate the work in Kolibri to support this use case.

@kollivier
Copy link
Contributor

@rtibbles I'm in the process of working up a response to your concerns, but I don't want that to hold this change up, as this is useful for other things as well, most notably pulling down channels to locally performance test Studio changes that affect them. (Or, as I'm in the process of working on for CK-12, tree diffing to know what changed or what may have broke when re-cheffing.) Also want to focus on getting the other performance fixes in ASAP, so expect something from me maybe late week, latest early next week. Also, I will briefly say I wouldn't consider this a concerted effort at this point, but I do think there's a growing feeling this may be needed to create a reliable experience for some groups that need to use Studio.

@jayoshih Thanks for adding the tests! I don't know why the codecov report didn't get posted for your latest commit, but I checked it on the codecov site and it says this puts us over 80% Python coverage! Woohoo! 💯(okay, so - 20%, but we're getting closer! ;-)

Copy link
Contributor

@kollivier kollivier left a comment

Choose a reason for hiding this comment

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

LGTM! :)

@kollivier kollivier merged commit 61ff12e into learningequality:develop Jul 8, 2019
@rtibbles
Copy link
Member

rtibbles commented Jul 9, 2019

@kollivier - yes this was not intended as a blocker to this PR at all, I just wanted to push back on the growing feeling I am sensing, as I don't think this is the solution to the issues we are seeing. Happy to continue the conversation outside of this PR!

@jayoshih jayoshih deleted the perseus-restore branch March 9, 2020 18:41
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.

None yet

3 participants