pylti / lti

Learning Tools Interoperability for Python
Other
78 stars 45 forks source link

LTI 2 #31

Closed ryanhiebert closed 6 years ago

ryanhiebert commented 8 years ago

Here's a high-level overview of the stuff that LTI 2 brings to the table:

https://www.imsglobal.org/lti-v2-introduction

subssn21 commented 7 years ago

I have been working on Implementing the Content Item Specification for my application and wanted to run a few things by people as they pertain to LTI 2 as well.

The Content Item Specification as well as much of LTI 2 makes use of JsonLD an extension to JSON. There are two python libraries that implement the extension.

https://github.com/digitalbazaar/pyld

https://github.com/RDFLib/rdflib-jsonld

pyLD seems to be more pythonic and has no other dependencies while rdflib-jsonld is dependent on rdflib, so my recommendation is that we use pyLD. I was hoping that the use of JSON LD would be fairly trivial and we could just build json documents with @ signs in a few places and be good, but after looking through tome of the LTI 2 documentation they do make use of some of the Json LD features.

The other thing i wanted to bring up related to LTI 2 is that there are a number of new message types, currently the LaunchParams class validates the keys globally, it may be a good idea to change that to validate based on the message type.

ryanhiebert commented 7 years ago

Thanks for the additional write-up here. Do you think that JSON-LD support is required before we can say this library supports LTI 2 launches? From what I read, it seemed like that was used for features that we don't yet implement. Obviously we can't call ourselves a full LTI 2 spec until then, but I'm trying to identify baby steps.

All things being equal otherwise, I agree with your motivation for choosing pyld over rdflib-jsonld. I don't think that we need and rdflib stuff, so it'd be weird to depend on that.

subssn21 commented 7 years ago

I am getting close to getting the Content Item Selection working. I will open a separate issue for that momentarily, but after working with it, It is possible that we may not have to address the Json data directly and therefore not worry about JSON-LD for the moment.

The way I have it implemented at the moment follows the current pattern used in other parts of the library. The new features I added just take the launch parameters and handle them, so at the moment it requires that the library user generate the appropriate data.

At some point it may be desirable to have some code to help with generating the appropriate JSON data like there is a helper for generating the XML data for outcomes.

subssn21 commented 7 years ago

I have been working on LTI 2 to get the Registration working. and I ran into some issues I wanted to get comment on:

In LTI2 many Launch Params are gone, including: context_id, contexttitle, lis*, resource_link_title, roles, user_id

It is customary, but not required to use the old names as custom* when setting parameters in resource handlers in LTI 2. Do we want try and automatically check for custom versions of these launch params? Part of me say we should, to keep the rework required for getting params to a minimum. Part of me says we shouldn't since it is a custom and not part of the spec.

One of the issues this creates is there is a function in ToolBase called has_role that depends on the existence of the roles launch param which no longer exists in LTI 2. There are three ways I can think of handling this:

  1. deprecate has_role and its dependent functions and expect users of the library to implement their own role code. (This is what I favor)

  2. Leave it and have it return an error if lti_version is 2.

  3. Have a way to tell the ToolBase what launch params to check for the role.

subssn21 commented 7 years ago

Do we want to try and build helpers or some other utility code around the JSON being passed into and out of the various new LTI calls?

We should find a way to make it more generic to call the various services for LTI 2 (See the ToolProxy class to see an example of what needs to be done to call a service)

ryanhiebert commented 7 years ago

One of the issues this creates is there is a function in ToolBase called has_role that depends on the existence of the roles launch param which no longer exists in LTI 2.

Luckily, this isn't correct. https://www.imsglobal.org/specs/ltiv2p0/implementation-guide#toc-39

LTI 2 roles do make things more complicated, though, because they define a different way to specify the same roles, and some of the new roles are ambiguous for attempting to convert to the old roles. It's annoying. The params branch may be instructional for learning a bit more about what I mean.

In LTI2 many Launch Params are gone, including: context_id, contexttitle, lis*, resource_link_title, roles, user_id

I'll note that LTI doesn't say that parameters not specified are not allowed to be present. It merely doesn't define what they mean. And of course it doesn't promise that there won't be a conflict in the future, where if you use custom_ or ext_ namespaced parameters it promises to not conflict.

In my opinion, it's the sanest approach to allow all launch parameters, specified by any version of LTI, for all LTI versions. Technically we could allow arbitrary parameters, but I don't know that that would actually be helpful, since it would mask more subtle errors, such as parameter name misspellings.


As far as utility code, that's fine with me, especially once it's proven that it'll be used in more than one place.

ryanhiebert commented 7 years ago

I actually think that it would be unwise for a tool consumer to add unrequested custom_ parameters. They've specified a mechanism for requesting those, and if they aren't going to be requested, they really need to be put into ext_ instead.

ryanhiebert commented 6 years ago

Looks like LTI 2 is being abandoned by IMS. Feel free to let me know if I've got that wrong, but unless that seems to change, I see no reason for our library to implement it either.

https://mfeldstein.com/ims-failed-lti-2-0/

subssn21 commented 6 years ago

So I think you are right. From the IMS Global website I was able to find this document that explains things best.

https://www.imsglobal.org/lti-adoption-roadmap

So I would agree that we should close LTI 2 feature, and work on removing LTI 2 related code and figure out what part of it is needed for 1.3(Advantage) (The oAuth 2 stuff I am sure as I know it supports that.)

On Fri, Jun 8, 2018 at 1:29 PM, Ryan Hiebert notifications@github.com wrote:

Looks like LTI 2 is being abandoned by IMS. Feel free to let me know if I've got that wrong, but unless that seems to change, I see no reason for our library to implement it either.

https://mfeldstein.com/ims-failed-lti-2-0/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pylti/lti/issues/31#issuecomment-395831333, or mute the thread https://github.com/notifications/unsubscribe-auth/AAsBk20Aw_IRDxGMVuaK4NpL0mi_un0Cks5t6rRogaJpZM4JWQoG .

-- Michael Robellard (216)288-2745