Jython 2.7 compatibility

[tommy-gilligan]

Currently Nodel runs on Jython 2.5.4-rc1. This is a bit limiting when you try to use 3rd party code in your recipe.

These changes make it possible to use Jython 2.7 instead (expanding what is possible in your recipes).

Leaving it as something to opt-in to for now: you can use Jython 2.5.4-rc1 or Jython 2.7 with these changes in place.
This means there is a bit of branching to do the right thing depending on which version of Jython you are trying to use. This is maybe not such a huge problem going forward because it doesn't look like there'll be a newer version of Jython any time soon.

[tommy-gilligan]

There's maybe a nicer way of getting a boolean back from the interpreter but AFAICT this works.

[justparking]

Hey @tomgilligan, nice one. I look forward to checking out what you've done and building the code.

It was a very very long time ago when I think I attempted something similar as a side project. I'm trying to remember exactly what the issue was, but there was one sticking point related to ... ummm I think, callbacks (function references) and them being implemented with Weak referencing in that version of Jython.

It would manifest itself as callbacks basically just going silent over a long period of time.

Anyway, it'd be good for you to run with it an see how long term tests go.

Oh yes, one other thing I noticed was it took MUCH longer to instantiate interpreter instances and deal with higher quantities of nodes.

So test for that too i.e. node count > 50, and see how it goes verses 2.5.4.

(@scroix FYI)

[justparking]

Hey @tomgilligan, I was expecting more affected files than just the PyNode.java file.

I thought this branch would be where Jython 2.7 itself is swapped in and gradle files would be affected, etc.

Is any of that work available too?

[tommy-gilligan]

Is any of that work available too?

Sorry, I really didn't word PR very well. I was kind of thinking of this as an opt-in change because I'm not sure how it would affect existing deployments. Maybe this is something that doesn't make sense to worry about depending on how much Nodel deployments get updated once they are in production. I will change it to default to 2.7.2.

Anyway, it'd be good for you to run with it an see how long term tests go.

I'll try and get some testing done this week.

[scroix]

I gave the binary a whirl. It looks like the toolkit is unavailable.

07-18 17:50:05.64 Traceback (most recent call last):
07-18 17:50:05.64   File "C:\Users\scroix\sandbox\ap\nodel\nodel-jython2.7\nodel-jyhost\build\distributions\standalone\.\nodes\test\script.py", line 15, in <module>
07-18 17:50:05.64     param_disabled = Parameter({'desc': 'Disables this node', 'schema': {'type': 'boolean'}})
07-18 17:50:05.64 NameError: name 'Parameter' is not defined
07-18 17:50:05.64 
07-18 17:50:05.64 (Python and Node script(s) loaded with errors (took 395ms); calling 'main' if present...)
07-18 17:50:05.64 (no 'main' to call)
07-18 17:50:35.12 Running Python 2.7.2 👍

It looks like it is there e.g. from sys import nodetoolkit executes, but there is some silent error and all API functions are unavailable, e.g. NameError: name 'Parameter' is not defined.

You can manually import it, but the "injection" seems to of broken.

from sys import nodetoolkit
nodetoolkit.systemClockInMillis()
> 123456789

As opposed to standard toolkit usage.

system_clock()
> 123456789

[scroix]

There's a fix for the injection of the toolkit sitting in (tommy-gilligan#1) which I've been testing and noticed our first of presumably many issues.

In Jython 2.5, if you tried to access a missing key in a java.util.LinkedHashMap object, it would return None. However, in Jython 2.7, accessing a missing key in a LinkedHashMap object will raise a KeyError exception, just like in a regular Python dictionary.

from java.util import LinkedHashMap

m = LinkedHashMap()
m.put("a", 1)

print(m.get("b"))

In Jython 2.5, this would print None, while in Jython 2.7 it would raise a KeyError exception with the message "KeyError: 'b'".

This behaviour occurs in the Calendar recipe as it parses members (traverse(memberInfo)), and there might be others we'll need to work through.

For example, it appears that In Jython 2.5, the string representation of a LinkedHashMap object used the {key=value} format, while in Jython 2.7, the string representation uses the {'key': 'value'} format.

[justparking]

Hey @scroix , that dictionary behaviour is one I've thought about for a while and would be nice to address. But as you say, it's a definite possible breaking change for existing recipes that have coding not expecting myDict['missingKey'] ... to throw an exception.

But regarding 2.7 vs 2.5, have you done any benchmarks regarding load and compile time for the same set of nodes?

I started doing that but then got side-tracked.

Loading / compiling times aside and a few class behaviour issues, have you noticed anything else?