Details

    • Rank:
      19500

      Description

      Place holder, more info to some.

        Issue Links

          Activity

          Mark Nielsen created issue -
          Mark Nielsen made changes -
          Hide
          Martin Dougiamas added a comment -

          I really hope this can be integrated for 2.2.

          I'm including part of an email from Kris Stokking at Moodlerooms:

          We've completed development for the Common Cartridge Import/Export upgrade to Moodle 2, and would like to share the code with you for (hopefully!) eventual inclusion into the core product. The implementation converts the available cartridges hosted by IMS and we anticipate it will be certified as IMS CC 1.0 and 1.1 compliant. We have the code hosted on github and opened a related tracker ticket at MDL-29956. The architecture should be sound and consistent with the Mark and Darko's original design with help from Eloy. Fortunately, the design allowed us to leverage much of the original code from 1.9. All in all, I think you will find the CC backup/restore routines to be very clean and non-invasive to the existing code.

          I would like to point out that the code is still going through QA cycles, so we anticipate there will be some minor changes made as a result. The documentation we have around CC also needs to be updated based on the development and QA effort, but I would be happy to share the Product Specs, the Developer docs or both in their current status if you think that would be helpful. In addition, I know that in some older conversations around CC for 2.X it was suggested that it be released as an experimental feature - we don't have the option in the current patch, but we could add it very easily as there are very few modifications made to core files. So some polish may still be needed, but it's nothing that couldn't be handled in a STABLE sprint and I know that we are all eager to get the CC code included into the next version of Moodle.

          Show
          Martin Dougiamas added a comment - I really hope this can be integrated for 2.2. I'm including part of an email from Kris Stokking at Moodlerooms: We've completed development for the Common Cartridge Import/Export upgrade to Moodle 2, and would like to share the code with you for (hopefully!) eventual inclusion into the core product. The implementation converts the available cartridges hosted by IMS and we anticipate it will be certified as IMS CC 1.0 and 1.1 compliant. We have the code hosted on github and opened a related tracker ticket at MDL-29956 . The architecture should be sound and consistent with the Mark and Darko's original design with help from Eloy. Fortunately, the design allowed us to leverage much of the original code from 1.9. All in all, I think you will find the CC backup/restore routines to be very clean and non-invasive to the existing code. I would like to point out that the code is still going through QA cycles, so we anticipate there will be some minor changes made as a result. The documentation we have around CC also needs to be updated based on the development and QA effort, but I would be happy to share the Product Specs, the Developer docs or both in their current status if you think that would be helpful. In addition, I know that in some older conversations around CC for 2.X it was suggested that it be released as an experimental feature - we don't have the option in the current patch, but we could add it very easily as there are very few modifications made to core files. So some polish may still be needed, but it's nothing that couldn't be handled in a STABLE sprint and I know that we are all eager to get the CC code included into the next version of Moodle.
          Martin Dougiamas made changes -
          Integrator stronk7
          Fix Version/s 2.2 [ 10656 ]
          Peer reviewer stronk7
          Priority Minor [ 4 ] Critical [ 2 ]
          Assignee Eloy Lafuente (stronk7) [ stronk7 ] Kris Stokking [ kstokking ]
          Mark Nielsen made changes -
          Labels moodlerooms partner
          Hide
          Martin Dougiamas added a comment -

          Eloy, I'm assuming you have this in your sights for today/tomorrow.

          Show
          Martin Dougiamas added a comment - Eloy, I'm assuming you have this in your sights for today/tomorrow.
          Martin Dougiamas made changes -
          Status Open [ 1 ] Waiting for integration review [ 10010 ]
          Martin Dougiamas made changes -
          Link This issue blocks MDL-30160 [ MDL-30160 ]
          Eloy Lafuente (stronk7) made changes -
          Currently in integration Yes [ 10041 ]
          Hide
          Mark Nielsen added a comment -

          Just updated the Diff URL with a change from Darko.

          Show
          Mark Nielsen added a comment - Just updated the Diff URL with a change from Darko.
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Hide
          Martin Dougiamas added a comment -

          Eloy, is this going to land by tomorrow?

          Show
          Martin Dougiamas added a comment - Eloy, is this going to land by tomorrow?
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Hi all,

          I've been reviewing all this along the last days, and it's great code but have found some problems that, after a lot of thinking and sharing it with Martin have leaded to the next conclusions:

          1) CC-Import: It's ok to send it upstream. It uses current support for backup converters in a proved way (1.9 => 2.x is using that already), integrates perfectly in the restore process and uses a lot of the logic that was present in the 1.9 importer. Also, it's somehow a bug-fix as far as restitutes one functionality we lost in 2.x.

          2) CC-Export: This cannot be integrated right now. It is new code, leading to one integration with the backup process that needs to be planned/discussed/agreed a bit more. This is a (cool) new feature, but it's not ready for the masses and, right now, roughly 2-3 weeks to release, is not the time to fix it.

          So, the immediate plan to get as much as possible of this landing is:

          1) Create one new $CFG->experimentalccexport setting, without UI, only available manually @ config.php that will "enable" cc-export, so backup won't get any change by default. This is the optimal solution, so all the code will land already in core and we'll be able to work there after release and will enable devs and other sites to use it as experimental feature if wanted.

          2) If I cannot isolate and get 1) done, I'll fallback to a more radical solution, deleting all the cc-export code interfering with "normal" backup. Then, after release, we can re-introduce it safely in future master branch (that will be 2.3 in 6 months).

          And that's the plan for NOW. I'm working on 1) above right now aiming to release 2.2beta in hours.

          Once again, I want to insist that I've liked the ideas/code I've seen, absolutely. Great job, guys!

          Ciao, Eloy

          PS: Side note, we would need some MDQLA tests covering CC-Import, do you've something about that (packages, tests...) we can incorporate to our MDLQA iterations?

          PS2: While trying cc-import I detected that some resources were not imported properly, leading to "file not found" errors, it was using the well-known "OrganicChemistry.zip" 1.0 package. (seem to be file-related problems both in file resources and question images)

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Hi all, I've been reviewing all this along the last days, and it's great code but have found some problems that, after a lot of thinking and sharing it with Martin have leaded to the next conclusions: 1) CC-Import: It's ok to send it upstream. It uses current support for backup converters in a proved way (1.9 => 2.x is using that already), integrates perfectly in the restore process and uses a lot of the logic that was present in the 1.9 importer. Also, it's somehow a bug-fix as far as restitutes one functionality we lost in 2.x. 2) CC-Export: This cannot be integrated right now. It is new code, leading to one integration with the backup process that needs to be planned/discussed/agreed a bit more. This is a (cool) new feature, but it's not ready for the masses and, right now, roughly 2-3 weeks to release, is not the time to fix it. So, the immediate plan to get as much as possible of this landing is: 1) Create one new $CFG->experimentalccexport setting, without UI, only available manually @ config.php that will "enable" cc-export, so backup won't get any change by default. This is the optimal solution, so all the code will land already in core and we'll be able to work there after release and will enable devs and other sites to use it as experimental feature if wanted. 2) If I cannot isolate and get 1) done, I'll fallback to a more radical solution, deleting all the cc-export code interfering with "normal" backup. Then, after release, we can re-introduce it safely in future master branch (that will be 2.3 in 6 months). And that's the plan for NOW. I'm working on 1) above right now aiming to release 2.2beta in hours. Once again, I want to insist that I've liked the ideas/code I've seen, absolutely. Great job, guys! Ciao, Eloy PS: Side note, we would need some MDQLA tests covering CC-Import, do you've something about that (packages, tests...) we can incorporate to our MDLQA iterations? PS2: While trying cc-import I detected that some resources were not imported properly, leading to "file not found" errors, it was using the well-known "OrganicChemistry.zip" 1.0 package. (seem to be file-related problems both in file resources and question images)
          Eloy Lafuente (stronk7) made changes -
          Labels moodlerooms partner docs_required moodlerooms partner qa_test_required
          Hide
          Darko Miletic added a comment -

          If you have any questions about the internals of the code and it's structure feel free to ask me directly.

          Show
          Darko Miletic added a comment - If you have any questions about the internals of the code and it's structure feel free to ask me directly.
          Eloy Lafuente (stronk7) made changes -
          Hide
          Kris Stokking added a comment -

          Eloy, I think that is a great approach. Thank you very much for your efforts, and if there's anything we can do to assist in #1, please let us know.

          Regarding the MDLQA tests, we are putting the CC code through IMS validation testing this week and can share the full testing plan with you by the end of the week. Will that be enough time for QA?

          Regarding the OrganicChemistry.zip, we'd be happy to include that in our testing - can you share a URL?

          Show
          Kris Stokking added a comment - Eloy, I think that is a great approach. Thank you very much for your efforts, and if there's anything we can do to assist in #1, please let us know. Regarding the MDLQA tests, we are putting the CC code through IMS validation testing this week and can share the full testing plan with you by the end of the week. Will that be enough time for QA? Regarding the OrganicChemistry.zip, we'd be happy to include that in our testing - can you share a URL?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Uhm, I think it is (was) one of the sample cartridges available @ the IMS CC/LTI Alliance site (for demoing IMS-CC 1.0?) Just have checked it now and the examples page seems to be under re-construction.

          In case you have not it, I think it's oki if I send you it privately (after all both us are members of such alliance).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Uhm, I think it is (was) one of the sample cartridges available @ the IMS CC/LTI Alliance site (for demoing IMS-CC 1.0?) Just have checked it now and the examples page seems to be under re-construction. In case you have not it, I think it's oki if I send you it privately (after all both us are members of such alliance). Ciao
          Hide
          Kris Stokking added a comment -

          If it's on the IMS CC Alliance site we'll be including that in our testing this week - thanks for the heads up!

          Show
          Kris Stokking added a comment - If it's on the IMS CC Alliance site we'll be including that in our testing this week - thanks for the heads up!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Done, I've pushed this to integration.git repo with one extra commit that implements support for:

          $CFG->enablebackupconverters = true;

          (without it, the IMS-CC export option is completely disabled. With it, it's enabled)

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Done, I've pushed this to integration.git repo with one extra commit that implements support for: $CFG->enablebackupconverters = true; (without it, the IMS-CC export option is completely disabled. With it, it's enabled) Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Doing a few tests before considering this integrated and test passed. MDLQA & conformance tests should be enough (although don't forget the problems detected with the package above).

          Then, I'm going to create one new backup: ims-cc component, adding to Darko as leader (if that is ok) in order to use it to have all the ims-cc issues under control.

          Finally, one new issue will be created about to discuss and agree the backup exporters infrastructure support (that IMS-CC export should be using).

          That's all... coming back in some minutes... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Doing a few tests before considering this integrated and test passed. MDLQA & conformance tests should be enough (although don't forget the problems detected with the package above). Then, I'm going to create one new backup: ims-cc component, adding to Darko as leader (if that is ok) in order to use it to have all the ims-cc issues under control. Finally, one new issue will be created about to discuss and agree the backup exporters infrastructure support (that IMS-CC export should be using). That's all... coming back in some minutes... ciao
          Eloy Lafuente (stronk7) made changes -
          Component/s Backup: IMS-CC [ 11735 ]
          Eloy Lafuente (stronk7) made changes -
          Link This issue testing discovered MDL-30265 [ MDL-30265 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I've added one more commit fixing some whitespace (tabs) and basic-tested it, both import and export and with the setting above enabled and disabled. Everything seems to be going ok.

          Also, I've created the "Backup: IMS-CC" component with Darko as leader.
          Finally, MDL-30265 is the issue where all the post-2.2-release work should happen.

          So, marking this as integrated and tested, yay!

          Ciao

          PS: Please, report any incidence with the IMS test suite and also anything detected by testing, reviewing as new issues here in the Tracker.

          Show
          Eloy Lafuente (stronk7) added a comment - I've added one more commit fixing some whitespace (tabs) and basic-tested it, both import and export and with the setting above enabled and disabled. Everything seems to be going ok. Also, I've created the "Backup: IMS-CC" component with Darko as leader. Finally, MDL-30265 is the issue where all the post-2.2-release work should happen. So, marking this as integrated and tested, yay! Ciao PS: Please, report any incidence with the IMS test suite and also anything detected by testing, reviewing as new issues here in the Tracker.
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Tester stronk7
          Eloy Lafuente (stronk7) made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks! Ciao
          Eloy Lafuente (stronk7) made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 15/Nov/11
          Hide
          Helen Foster added a comment -

          Anyone have a sample IMS CC package which we can use for QA testing IMS CC import?

          Also if anyone has any feedback on MDLQA-1426...

          Show
          Helen Foster added a comment - Anyone have a sample IMS CC package which we can use for QA testing IMS CC import? Also if anyone has any feedback on MDLQA-1426 ...
          Helen Foster made changes -
          Labels docs_required moodlerooms partner qa_test_required docs_required moodlerooms partner
          Helen Foster made changes -
          Labels docs_required moodlerooms partner moodlerooms partner

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: