search

UniversalDevicesInc / Polyglot

Polyglot Open Source
MIT License
24 stars 2 forks source link

API/ISY version information should be available to node servers #8

Closed mjwesterhof closed 8 years ago

mjwesterhof commented 8 years ago

It is desirable that the Polyglot framework know about and communicate version information for all the "moving parts". At the least, the information should appear in the logs for ease of diagnostics, but preferably this information should be made available to the node server so that the node server could adjust its behavior. For example, a node server may choose to use certain messages only if it knows that the Polyglot framework and the ISY are at least a certain version.

The means for communicating this are yet to be defined, but one possibility is to add information to the ping or config messages that would include the version information for the ISY and the Framework.

Einstein42 commented 8 years ago

brach issue-8 currently stores the ISY version from /rest/config pull to configuration.json. I will extrapolate this into a method callable from SimpleNodeServer extractions to pull from config file instead of calling the web service every time. This will reduce network overhead as polyglot is already noisy.

mkohanim commented 8 years ago

Hi James,

This is an excellent idea. Thanks.

With kind regards,

Michel Kohanim CEO

(p) 818.631.0333http://tel:818.631.0333 (f) 818.436.0702http://tel:818.436.0702 http://www.universal-devices.comhttp://www.universal-devices.com/

On March 15, 2016 11:00:44 PM James notifications@github.com wrote:

brach issue-8 currently stores the ISY version from /rest/config pull to configuration.json. I will extrapolate this into a method callable from SimpleNodeServer extractions to pull from config file instead of calling the web service every time. This will reduce network overhead as polyglot is already noisy.

You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHubhttps://github.com/UniversalDevicesInc/Polyglot/issues/8#issuecomment-197165866

Einstein42 commented 8 years ago

Ok. This probably needs to be tested by more than just me. I've loaded my node server as well as the kodi and hue node servers and verified that I can pull the version from all three just using the 'self.poly.isyver' variable from the SimpleNodeServer class (or NodeServer). I don't want to merge this until I get a confirmation that no one is having issues. If the ISY is not connectable then it just passes "" if the ISY is unreachable, we handle the exceptions and proceed normally.

jimboca commented 8 years ago

I'll test it when I get a chance, but probably won't be for a few days, as long as you can test my issue-1 branch with your node server and hue :)

Einstein42 commented 8 years ago

I don't have a hue, but maybe I should just pick one up since we are doing so much work on that in particular. I'll see what I can do.

jimboca commented 8 years ago

Haha, I was thinking the same thing. Doesn't any on here have a Hue?

mjwesterhof commented 8 years ago

Have Hue. Lack time! :-) Just picked up a GE Link bulb, so I'll work on a patch for that problem -- and I'll just base that work on this same code branch, and test all the changes together. This weekend, I hope...

jimboca commented 8 years ago

Should we create a dev branch and merge both our changes into that one for you to test on? I'm not totally sure the recommended way to do this with git/github so if you all have a preferred method we should move to that?

mkohanim commented 8 years ago

Hi Jim,

I think having a development branch would be a good idea. Then we can merge back to the master branch.

With kind regards,

Michel Kohanim CEO

(p) 818.631.0333 (f) 818.436.0702 http://www.universal-devices.comhttp://www.universal-devices.com/

From: jimboca [mailto:notifications@github.com] Sent: Thursday, March 17, 2016 12:53 PM To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com Cc: Michel Kohanim michel@universal-devices.com Subject: Re: [Polyglot] API/ISY version information should be available to node servers (#8)

Should we create a dev branch and merge both our changes into that one for you to test on? I'm not totally sure the recommended way to do this with git/github so if you all have a preferred method we should move to that?

— You are receiving this because you commented. Reply to this email directly or view it on GitHubhttps://github.com/UniversalDevicesInc/Polyglot/issues/8#issuecomment-198056159

mkohanim commented 8 years ago

I just created “development” branch.

With kind regards,

Michel Kohanim CEO

(p) 818.631.0333 (f) 818.436.0702 http://www.universal-devices.comhttp://www.universal-devices.com/

From: jimboca [mailto:notifications@github.com] Sent: Thursday, March 17, 2016 12:53 PM To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com Cc: Michel Kohanim michel@universal-devices.com Subject: Re: [Polyglot] API/ISY version information should be available to node servers (#8)

Should we create a dev branch and merge both our changes into that one for you to test on? I'm not totally sure the recommended way to do this with git/github so if you all have a preferred method we should move to that?

— You are receiving this because you commented. Reply to this email directly or view it on GitHubhttps://github.com/UniversalDevicesInc/Polyglot/issues/8#issuecomment-198056159

jimboca commented 8 years ago

Ok, I merged in changes for issue-1 on the development branch so you can test that both work. There was a conflict between #1 and #15 changes, but I think I fixed them correctly.

Einstein42 commented 8 years ago

I went ahead and merged branch issue-8 into development and removed that branch. Jimboca you have a note in the readme that the python -m polyglot -v doesn't work for python2, I think you fixed that did you not?

jimboca commented 8 years ago

Yes, it works now. No idea what my problem was before

Einstein42 commented 8 years ago

This is functional and tested.

polyglot.nodeserver_manager: Hu STDIN: {"isyver": {"version": "5.0.2"}}

Accessible by: self.poly.isyver from SimpleNodeServer(NodeServer)

mjwesterhof commented 8 years ago

Ok, this half of things is working - thanks! A node server can now, for example, alter its behavior to match the capabilities of a given ISY firmware that it is servicing.

We now have to figure out the other half - which is probably simpler to implement from a technical point of view. A node server needs to be able to determine the version of the API between Polyglot and the node server itself. The reason we need this is actually best exemplified by the recent breaking changes made in Issue #1 -- I now have two very subtly different versions of a very simple node server, one that works with the original released version of Polyglot, and one that works on the development branch. But I don't have any means at all for either a user or the node server code to determine which of the two versions should be used.

I'd suggest that just adding a JSON value to the CONFIG API should be sufficient for changes to the API (the one that's read/written to STDIN/STDOUT). We probably need another value to represent breaking changes to the nodeserver_api.py interface. And yes, that's a lot of version numbers, but integers are cheap!

Other suggestions welcome.

jimboca commented 8 years ago

I agree we should have a version number for the polyglot API to check for any issues like this.

For now, in your NodeServer you should be able to use this:

        if hasattr(self,'_is_node_server'):

Which will tell you that it's the latest version.

I'm not sure if it needs to be in the config, or if it should just be a constant in the code.

mjwesterhof commented 8 years ago

I'll test that code in the node server - a little kludgy, but better than nothing.

A constant will work for consumers of the nodeserver_apy.py interface - a different version number needs to be in place for consumers of the STDIN/STDOUT interface such as my Perl node server. While it is less likely that we'll ever make a breaking change to that API, we've already added a number of messages (isyver, sandbox, for example) so it's only a matter of time before a consumer of that API will find a version number to be useful. I am suggesting we embed that into the config message because that is the first message sent, and the recommended best practice documented is to have a node server wait for that message when it starts.

Einstein42 commented 8 years ago

Sorry guys, life has had me a little busy lately. What do you think about version numbering? 0.1.?

mkohanim commented 8 years ago

Hi James, sounds good to me.

Also, does anyone on the list have Sonos? If so, please contact me directly to see whether or not we can add Sonos to Polyglot on fee basis. Thank you.

With kind regards,

Michel Kohanim CEO

(p) 818.631.0333 (f) 818.436.0702 http://www.universal-devices.comhttp://www.universal-devices.com/

From: James [mailto:notifications@github.com] Sent: Monday, March 28, 2016 6:50 AM To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com Cc: Michel Kohanim michel@universal-devices.com Subject: Re: [Polyglot] API/ISY version information should be available to node servers (#8)

Sorry guys, life has had me a little busy lately. What do you think about version numbering? 0.1.?

— You are receiving this because you commented. Reply to this email directly or view it on GitHubhttps://github.com/UniversalDevicesInc/Polyglot/issues/8#issuecomment-202402023

Einstein42 commented 8 years ago

I sent you a message. I have it.

mjwesterhof commented 8 years ago

Regarding version numbers:

Executive summary: For Polyglot itself, 1.0.n where n is incremented with each commit to mainline. For internal APIs, a simple integer that is incremented whenever said API is changed.

Details:

For Polyglot itself I don't think we have a version number in place yet -- we can certainly think of the first version that was published at 1.0.0 though. So we'll need to add that as a string to the main routine, and arrange for that to be printed at startup to the log file at the very least. As for the version numbering scheme going forward, I think the typical major.minor.bugfix numbering is pretty much expected by users. IMO, the bugfix number should be incremented on commit to mainline (which will usually be a merge from one or another development branch). But I'm quite willing to accept other options.

For versioning of APIs, such as the nodeserver_api.py interface and for the STDIN/STDOUT messaging API, those are intended for code, not for humans so much -- a simple integer (incremented whenever a significant new feature or breaking change is introduced to that API) will be ideal. That way the node server code can do a very simple integer greater-than or less-than comparisons. For example, my node server might like to test that version to see if the user's Polyglot instance supports some new feature, or even print a warning or informational message to the log if the Polyglot server's API is newer than the API version that the specific node server was tested against.

(BTW, ultimately I'd like to see versioning extended to the profile.zip as well, but this would require ISY-side changes. It would be great if a node server can query the ISY (via polyglot of course) to determine what version of its matching profile is installed in the ISY. This would allow a node server to first of all determine if ANY version of the profile is installed, and print the appropriate diagnostic to the log -- right now there's no way for the node server or even the human being to know if the profile got installed or if there was a syntax error, or what! Secondly, this would allow the node server to perhaps "limp along" with an older profile until the user can find a convenient time to reboot the ISY so that it installs the newer profile. Finally, even if the node server itself does nothing with this information, just having the version number of the profile in the Polyglot log file would be incredibly useful for the developer trying to support a user who is having troubles of some sort.)

(edited - github ate my greater-than and less-than symbols. :(

mkohanim commented 8 years ago

Hi Mike,

This is excellent. I we should use what we already use which is very similar to node.js: Major.Minor.Revision Revision changes will have no impact on existing code (be it API or otherwise): i.e. everything will continue to work Minor changes most probably will have no impact but the “developer” and “consumer” should read the release notes and make sure there’s no impact Major changes most probably will have impact

With regards to versioning Profiles.zip, I am discussing it with Chris.

And, I want to thank you all for all your contributions to this very important project which brings IoT to ISY. As such, please do let me know if you need any of our products and modules and I would be delighted to accommodate free of charge.

Thanks again SO very much!

With kind regards,

Michel Kohanim CEO

(p) 818.631.0333 (f) 818.436.0702 http://www.universal-devices.comhttp://www.universal-devices.com/

From: Mike Westerhof [mailto:notifications@github.com] Sent: Monday, March 28, 2016 8:55 AM To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com Cc: Michel Kohanim michel@universal-devices.com Subject: Re: [Polyglot] API/ISY version information should be available to node servers (#8)

Regarding version numbers:

Executive summary: For Polyglot itself, 1.0. where n is incremented with each commit to mainline. For internal APIs, a simple integer that is incremented whenever said API is changed.

Details:

For Polyglot itself I don't think we have a version number in place yet -- we can certainly think of the first version that was published at 1.0.0 though. So we'll need to add that as a string to the main routine, and arrange for that to be printed at startup to the log file at the very least. As for the version numbering scheme going forward, I think the typical .. numbering is pretty much expected by users. IMO, the number should be incremented on commit to mainline (which will usually be a merge from one or another development branch). But I'm quite willing to accept other options.

For versioning of APIs, such as the nodeserver_api.py interface and for the STDIN/STDOUT messaging API, those are intended for code, not for humans so much -- a simple integer (incremented whenever a significant new feature or breaking change is introduced to that API) will be ideal. That way the node server code can do a very simple integer greater-than or less-than comparisons. For example, my node server might like to test that version to see if the user's Polyglot instance supports some new feature, or even print a warning or informational message to the log if the Polyglot server's API is newer than the API version that the specific node server was tested against.

(BTW, ultimately I'd like to see versioning extended to the profile.zip as well, but this would require ISY-side changes. It would be great if a node server can query the ISY (via polyglot of course) to determine what version of its matching profile is installed in the ISY. This would allow a node server to first of all determine if ANY version of the profile is installed, and print the appropriate diagnostic to the log -- right now there's no way for the node server or even the human being to know if the profile got installed or if there was a syntax error, or what! Secondly, this would allow the node server to perhaps "limp along" with an older profile until the user can find a convenient time to reboot the ISY so that it installs the newer profile. Finally, even if the node server itself does nothing with this information, just having the version number of the profile in the Polyglot log file would be incredibly useful for the developer trying to support a user who is havin g troubles of some sort.)

— You are receiving this because you commented. Reply to this email directly or view it on GitHubhttps://github.com/UniversalDevicesInc/Polyglot/issues/8#issuecomment-202454689

mkohanim commented 8 years ago

Hello all, this is from Chris: “I agree we should have an API telling that status of the profile (whether there are errors, etc.). We do have APIs that allow any client (e.g. Admin Console) to download the profile files (including the version.txt), but I don’t think its in the formal documentation. The version.txt file is completely defined by the owner of the profile, and thus they should use that to manage versions. I’ll update the documentation.”

With kind regards,

Michel Kohanim CEO

(p) 818.631.0333 (f) 818.436.0702 http://www.universal-devices.comhttp://www.universal-devices.com/

From: Mike Westerhof [mailto:notifications@github.com] Sent: Monday, March 28, 2016 8:55 AM To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com Cc: Michel Kohanim michel@universal-devices.com Subject: Re: [Polyglot] API/ISY version information should be available to node servers (#8)

Regarding version numbers:

Executive summary: For Polyglot itself, 1.0. where n is incremented with each commit to mainline. For internal APIs, a simple integer that is incremented whenever said API is changed.

Details:

For Polyglot itself I don't think we have a version number in place yet -- we can certainly think of the first version that was published at 1.0.0 though. So we'll need to add that as a string to the main routine, and arrange for that to be printed at startup to the log file at the very least. As for the version numbering scheme going forward, I think the typical .. numbering is pretty much expected by users. IMO, the number should be incremented on commit to mainline (which will usually be a merge from one or another development branch). But I'm quite willing to accept other options.

For versioning of APIs, such as the nodeserver_api.py interface and for the STDIN/STDOUT messaging API, those are intended for code, not for humans so much -- a simple integer (incremented whenever a significant new feature or breaking change is introduced to that API) will be ideal. That way the node server code can do a very simple integer greater-than or less-than comparisons. For example, my node server might like to test that version to see if the user's Polyglot instance supports some new feature, or even print a warning or informational message to the log if the Polyglot server's API is newer than the API version that the specific node server was tested against.

(BTW, ultimately I'd like to see versioning extended to the profile.zip as well, but this would require ISY-side changes. It would be great if a node server can query the ISY (via polyglot of course) to determine what version of its matching profile is installed in the ISY. This would allow a node server to first of all determine if ANY version of the profile is installed, and print the appropriate diagnostic to the log -- right now there's no way for the node server or even the human being to know if the profile got installed or if there was a syntax error, or what! Secondly, this would allow the node server to perhaps "limp along" with an older profile until the user can find a convenient time to reboot the ISY so that it installs the newer profile. Finally, even if the node server itself does nothing with this information, just having the version number of the profile in the Polyglot log file would be incredibly useful for the developer trying to support a user who is havin g troubles of some sort.)

— You are receiving this because you commented. Reply to this email directly or view it on GitHubhttps://github.com/UniversalDevicesInc/Polyglot/issues/8#issuecomment-202454689

Einstein42 commented 8 years ago

Jimbo, I'm going to try to lump all of our additions to the stdin/stdout into second \'params\' message. We already have config, then we added isyver, sandbox and name. Now I'm going to have another for apiver. Seems cleaner to package those all up into a single params message. That way if we need to add more it can scale without being so noisy.

jimboca commented 8 years ago

Yes, I was thinking that as well :+1: After I added sandbox I thought instead we should have one that returns a hash.

Einstein42 commented 8 years ago

STDIN: {"params": {"apiver": "1.0.1", "isyver": "5.0.2", "sandbox": "/home/pi/dev/Polyglot/config/hue", "name": "Blah"}}

Ok I have made the change. It is currently in branch jamesdev

It shouldn't break any node servers as it is the same self. variables that they read from.

I tested with no issues, please verify before I merge into dev

Einstein42 commented 8 years ago

Also as an aside. I added a static APIVER at 1.0.1, increment if we add any changed that would impact client node_servers. This should resolve the entirety of this issue #8. Any other feedback before we move on?

mjwesterhof commented 8 years ago

No, this does not yet resolve #8 -- at the risk of sounding terse, let me be very specific:

a) Change the static APIVER name to PGVER, and the name apiver in the params message to pgver -- the value remains (currently) "1.0.1" Arrange, somewhere in the startup of Polyglot itself, for this value to be printed to the log file as the main Polyglot version number.

b) Add the static value PGAPIVER to polyglot, with the value "1", and add the name "pgapiver" with its value to the params message.

c) Add the static value NSAPIVER to nodeserver_apy.py, with the value "1".

These changes will accomplish what I outlined in my previous message on this issue.

(I'll be happy to do this myself, by the way, but I've not done so for fear of treading on the toes of those who are working on this issue - "too many cooks in the kitchen" is not a good thing.)

Einstein42 commented 8 years ago

I see... seems a bit excessive, but not complicated. I will make the changes you outlined and commit shortly.

mjwesterhof commented 8 years ago

I explained my reasoning several comments above - what part of that is extraneous or excessive? (I'm not being argumentative here, I'm asking because maybe I've missed something - if there is a reasonable way for an arbitrary node server to know what it can send and can expect to receive and how to do so that doesn't push the complexity into the node server itself, I'm completely happy with that!)

Einstein42 commented 8 years ago

polyglot.nodeserver_manager: Hue STDIN: {"params": {"pgver": "1.0.1", "name": "Hue", "pgapiver": "1", "nsapiver": "1", "sandbox": "/home/pi/dev/Polyglot/config/hue", "isyver": "5.0.2"}}

Commit added. Please verify functionality.

Einstein42 commented 8 years ago

I'm not entirely clear on why you would need nsapi vs pgapi. They are invariably going to be equal as you have to mirror the changes in each the nodeserver manager and nodeserver api. That being said, it really is irrelevant to the code, so having both is no big deal. Took 30 seconds to finish and test.

mjwesterhof commented 8 years ago

Thanks - this will be very useful for node server authors who don't care about the Polyglot framework itself.

Regarding the two api versions: actually nsapi and pgapi are quite different -- for example, the changes made for issue #1 broke consumers of the nodeserver_api.py interface since it changed the calling conventions for node creation, but had no impact at all on consumers of the STDIN/STDOUT messaging interface since none of the messages there had any breaking changes.

(BTW I have a pair of equivalent very simple node servers, one written in Python using nodeserver_apy.py and the other doing exactly the same thing but written in Perl and using the STDIN/STDOUT interface -- these have proven very useful for testing. I'll publish these somewhere when I get a few moments to add some comments to the code...)

Regarding the idea that we just use one version number, the Polyglot version, we could -- but that leads to code in each and every node server that looks like this pseudo-code:

(major, minor, bugfix) = split(isyver, ".") if ((major > "5" || (major == "5" && minor > "4") || (major == "5" && minor == "4" && bugfix > "3")) then Do something that requires at least Polyglot 5.4.3 else Do it the old way

My experience has been that the above code doesn't get written -- the developer, faced with such a structure, says "screw it! somebody can just put a note in the release notes 'cause I'm not going to code this check!". And of course, users don't read release notes even if they understood the technical detail therein, and service calls and irate users result (which is a bad thing, since we really need to consider Polyglot to be a commercial-grade product, IMO). In summary, by using a simple integer to version each api, we simplify life for consumers of those APIs, and thus encourage developers of those node servers to "do the right thing":

if (nsapy > 6) then Do it the new version 6 API way else Do it the old way

Einstein42 commented 8 years ago

Interesting. Thanks for the perspective. I'm on board with that.

mjwesterhof commented 8 years ago

Restructuring of some the details of this is required.

mjwesterhof commented 8 years ago

Usage comment (FYI) -- easy way for an arbitrary node server to determine if the hosting Polyglot is the original (released) version, or the current development head version (which has the logger enhancements and the params message with version information, etc).

In the main node server python file, add the following:

try:
    from polyglot.nodeserver_api import NS_API_VERSION
except ImportError:
    NS_API_VERSION = 0

The code can then test NS_API_VERSION -- if equal to 0 (integer, not string), then the hosting polyglot is the originally-published version, if > 0, then it's newer (and the version number will define the capabilities of the python nodeserver_api.py source file, and the content of the params message will define the version of the Polyglot package and the ISY on the other end of the network connection.)

mjwesterhof commented 8 years ago

Remaining TODO before this closed: update the documentation in the docs folder here in git. Need to document:

Einstein42 commented 8 years ago

https://github.com/UniversalDevicesInc/Polyglot/wiki/NodeServer-API-Additions-version-1.0.1

Params message was documented here. Does anyone know how to update the documentation in the docs folder?

mkohanim commented 8 years ago

No idea at all. I am going to have Brad take a look.

With kind regards,

Michel Kohanim CEO

(p) 818.631.0333 (f) 818.436.0702 http://www.universal-devices.comhttp://www.universal-devices.com/

From: James [mailto:notifications@github.com] Sent: Monday, April 4, 2016 9:44 AM To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com Cc: Michel Kohanim michel@universal-devices.com Subject: Re: [UniversalDevicesInc/Polyglot] API/ISY version information should be available to node servers (#8)

https://github.com/UniversalDevicesInc/Polyglot/wiki/NodeServer-API-Additions-version-1.0.1

Params message was documented here. Does anyone know how to update the documentation in the docs folder?

— You are receiving this because you commented. Reply to this email directly or view it on GitHubhttps://github.com/UniversalDevicesInc/Polyglot/issues/8#issuecomment-205387223

Einstein42 commented 8 years ago

Thanks Michel. Have him let us know when he figures it out. You are welcome to give him my email address. I'll post a tutorial on the wiki once we get that nailed down.

mkohanim commented 8 years ago

Michel,

You can edit the message at the link you provided by using "Edit" but I do not see a way to edit documents from the folders, I only see a way to view/download the raw file. Perhaps trying to upload a file with same name after editing will replace? I'm not 100% but there must be a way.

Brad

On Apr 4, 2016, at 10:59 AM, Michel Kohanim wrote: No idea at all. I am going to have Brad take a look.

With kind regards,

Michel Kohanim CEO

(p) 818.631.0333 (f) 818.436.0702 http://www.universal-devices.comhttp://www.universal-devices.com/

From: James [mailto:notifications@github.com] Sent: Monday, April 4, 2016 9:44 AM To: UniversalDevicesInc/Polyglot Polyglot@noreply.github.com<mailto:Polyglot@noreply.github.com> Cc: Michel Kohanim michel@universal-devices.com<mailto:michel@universal-devices.com> Subject: Re: [UniversalDevicesInc/Polyglot] API/ISY version information should be available to node servers (#8)

https://github.com/UniversalDevicesInc/Polyglot/wiki/NodeServer-API-Additions-version-1.0.1

Params message was documented here. Does anyone know how to update the documentation in the docs folder?

— You are receiving this because you commented. Reply to this email directly or view it on GitHubhttps://github.com/UniversalDevicesInc/Polyglot/issues/8#issuecomment-205387223

Brad McAdams Technical Sales Engineer (p) 818.631.0333 (f) 818.436.0702 http://www.universal-devices.com

mjwesterhof commented 8 years ago

I'm considering this one closed; re-open if anyone disagrees.