Re: svn commit: r1432778 - in /subversion/trunk/tools/server-side/svnpubsub: README.txt commit-hook.py daemonize.py example.conf irkerbridge.py notes/ svnpubsub/client.py svnpubsub/server.py svnwcsub.conf.example svnwcsub.py test.conf testserver.py watcher.py

[Branko Čibej <br...@wandisco.com>]

On 14.01.2013 02:16, breser@apache.org wrote:
> - Each JSON record is separated with a null character.

Why? If I understand this change, the result will be a not very standard
stream of nul-terminated JSON objects. While the svnpubsub client will
parse that, other JSON-consuming tools will need extra work.

Why not return a proper JSON array instead?

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com

Re: svn commit: r1432778 - in /subversion/trunk/tools/server-side/svnpubsub: README.txt commit-hook.py daemonize.py example.conf irkerbridge.py notes/ svnpubsub/client.py svnpubsub/server.py svnwcsub.conf.example svnwcsub.py test.conf testserver.py watcher.py

[Ben Reser <be...@reser.org>]

On Sun, Jan 13, 2013 at 10:46 PM, Branko Čibej <br...@wandisco.com> wrote:
> Why? If I understand this change, the result will be a not very standard
> stream of nul-terminated JSON objects. While the svnpubsub client will
> parse that, other JSON-consuming tools will need extra work.
>
> Why not return a proper JSON array instead?

Indeed that is exactly how it was.  The problem is that is a pain
(thought not impossible) to write a client for JSON with the old
format.

When I started writing irkerbridge I was unaware there was a client
and so I started just trying to use the JSON stream directly and I ran
into the same issues these guys did:
http://www.enricozini.org/2011/tips/python-stream-json/

Without thinking too much I started stripping the trailing ',' and
feeding the results into json.loads().  Then I realized that it
wouldn't work with a large log message that ends up going as being
split up and you get multiple blocks delivered to the application.

I used '\0' instead of '\n' as suggested on that page because a null
is not valid in JSON unless escaped so you can trivially transform the
whole thing into an array if you needed to.

The other alternatives are to find a JSON parser for python that
supports streaming (might be able to use the object_hook in the json
module to do this).  Or make the client code JSON aware and do
something like counting brackets to figure out when you're ready to
decode an object.

I chose this method because it made it easier for everyone to write a
client even if they chose some other language or toolset.  As a group
if we don't like that chose I'm happy to change it again.

Re: svn commit: r1432778 - in /subversion/trunk/tools/server-side/svnpubsub: README.txt commit-hook.py daemonize.py example.conf irkerbridge.py notes/ svnpubsub/client.py svnpubsub/server.py svnwcsub.conf.example svnwcsub.py test.conf testserver.py watcher.py

[Branko Čibej <br...@wandisco.com>]

On 14.01.2013 11:02, Daniel Shahaf wrote:
> Branko Čibej wrote on Mon, Jan 14, 2013 at 07:46:16 +0100:
>> On 14.01.2013 02:16, breser@apache.org wrote:
>>> - Each JSON record is separated with a null character.
>> Why? If I understand this change, the result will be a not very standard
>> stream of nul-terminated JSON objects. While the svnpubsub client will
>> parse that, other JSON-consuming tools will need extra work.
>>
>> Why not return a proper JSON array instead?
> That's what the code was doing originally: return commit records as one
> indefinite array.  That forced clients to use an iterative parsing
> approach, ie, needless complication.
>
> The current approach allows clients to loop on "read one complete json
> object" from the stream.

Thanks for the clarification.

-- Brane


-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com

Re: svn commit: r1432778 - in /subversion/trunk/tools/server-side/svnpubsub: README.txt commit-hook.py daemonize.py example.conf irkerbridge.py notes/ svnpubsub/client.py svnpubsub/server.py svnwcsub.conf.example svnwcsub.py test.conf testserver.py watcher.py

[Daniel Shahaf <d....@daniel.shahaf.name>]

Branko Čibej wrote on Mon, Jan 14, 2013 at 07:46:16 +0100:
> On 14.01.2013 02:16, breser@apache.org wrote:
> > - Each JSON record is separated with a null character.
> 
> Why? If I understand this change, the result will be a not very standard
> stream of nul-terminated JSON objects. While the svnpubsub client will
> parse that, other JSON-consuming tools will need extra work.
> 
> Why not return a proper JSON array instead?

That's what the code was doing originally: return commit records as one
indefinite array.  That forced clients to use an iterative parsing
approach, ie, needless complication.

The current approach allows clients to loop on "read one complete json
object" from the stream.

I suppose wrapping each record as a one-element array would serve the
same purpose as the NUL bytes: act as a belt-and-suspenders interrecord
delimiter (apart from the implied one by the matching } to the opening {
of a record).

Daniel