On 12/31/24 02:31, Greg Stein wrote:
My recommendation would be to patch the asfpy package to understand the
svnpubsub protocol (basically: \0 instead of \n). The asfpy.pubsub
package is what Infra uses for all of its clients to listen to the
official ASF pubsub service. It's fully async.
As far as I recall, asfpy's pubsub works with both \n and \x00 endings
out of the box[1]. I suppose you could test it on our svnpubsub stream
and find out?
[1]
https://github.com/apache/infrastructure-asfpy/blob/main/asfpy/pubsub.py#L218
Check out the pubsub.py module in https://github.com/apache/
infrastructure-asfpy/ <https://github.com/apache/infrastructure-asfpy/>
Once patched for "\0", then we should point people there for a pubsub
client. "asfpy" is a public package, maintained by the Foundation, and
very willing to accept patches. (and delete our own client; shifting the
server to asyncio is another beast; I'd recommend pointing people to
pypubsub at one of:
* https://github.com/apache/infrastructure-pypubsub <https://github.com/
apache/infrastructure-pypubsub> (downstream of:)
* https://github.com/Humbedooh/pypubsub <https://github.com/Humbedooh/
pypubsub>
It looks like some patches in the infra fork have not been merged upstream.
Cheers,
-g
On Sun, Dec 22, 2024 at 2:31 PM Daniel Sahlberg
<daniel.l.sahlb...@gmail.com <mailto:daniel.l.sahlb...@gmail.com>> wrote:
Thank you very much for the review! I'll answer inline below.
Den tis 3 dec. 2024 kl 22:31 skrev Jun Omae <jun6...@gmail.com
<mailto:jun6...@gmail.com>>:
Hi,
On 2024/11/29 6:47, Daniel Sahlberg wrote:
[snip]
>
> The next step will be to rewrite for asyncio, but I'd like to
have a functional client as a baseline.
>
> Cheers,
> Daniel
>
> [1] https://github.com/apache/infrastructure-pypubsub/blob/
master/pypubsub.py <https://github.com/apache/infrastructure-
pypubsub/blob/master/pypubsub.py>
I'm not familiar with svnpubsub.py but tried to run the script
with the
proposed patch.
However it doesn't work. Investigating it, 400 Bad Request is
received because
the client.py sends "GET HTTP/1.1" (missing request uri). After
applying the
following patch, it seems to work.
[[[
diff --git a/tools/server-side/svnpubsub/svnpubsub/client.py b/
tools/server-side/svnpubsub/svnpubsub/client.py
index 871a5e925..004003512 100644
--- a/tools/server-side/svnpubsub/svnpubsub/client.py
+++ b/tools/server-side/svnpubsub/svnpubsub/client.py
@@ -75,7 +75,7 @@ class Client(asynchat.async_chat):
raise ValueError("URL scheme must be http: '%s'" % url)
host = parsed_url.hostname
port = parsed_url.port
- resource = parsed_url.path
+ resource = parsed_url.path or '/'
if parsed_url.query:
resource += "?%s" % parsed_url.query
if parsed_url.fragment:
]]]
I think it would simplify by the following rather than `host +
":" + str(port)`.
[[[
- self.push(('GET %s HTTP/1.0\r\n\r\n' %
resource).encode('ascii'))
+ self.push(('GET %s HTTP/1.1\r\nHost: %s:%d\r\n\r\n' %
+ (resource, host, port)).encode('ascii'))
]]]
As Daniel Gruno noted, I was querying the wrong endpoint, so the
HTTP/1.1 protocol isn't really needed.
We might want to support both protocols at some point in time, but
I'll leave these two enhancements for the future.
In the minimal client, it uses `repr` but `__repr__` method of
`Notification`
class is not defined. I'd suggest to add the `__repr__` method.
[[[
@@ -131,6 +140,8 @@ class Client(asynchat.async_chat):
class Notification(object):
def __init__(self, data):
self.__dict__.update(data)
+ def __repr__(self):
+ return '%s(%r)' % (self.KIND, self.__dict__)
class Commit(Notification):
KIND = 'COMMIT'
]]]
Finally, `cc` and `mc` methods in the minimal client should have
`url` parameter.
[[[
--- svnpubsub-client.py.orig 2024-12-04 06:25:33.083708157 +0900
+++ svnpubsub-client.py 2024-12-02 10:28:30.489176287 +0900
@@ -1,10 +1,10 @@
-def cc(o):
+def cc(url, o):
print("commit " + repr(o))
def ec(url, type, o):
print("event on " + repr(url) + ": " + repr(type) + "\n" +
repr(o))
-def mc(o):
+def mc(url, o):
print("metadata " + repr(o))
import client
]]]
Good catch! With these two fixes, I now have a functioning client:
[[[
event on 'http://svn.apache.org:2069/commits/ <http://
svn.apache.org:2069/commits/>': 'ping'
1734899014.037649
event on 'http://svn.apache.org:2069/commits/ <http://
svn.apache.org:2069/commits/>': 'ping'
1734899029.038485
commit http://svn.apache.org:2069/commits/ <http://
svn.apache.org:2069/commits/>: COMMIT({'committer': 'dsahlberg',
'log': 'Add a __repr__ function to allow repr() a Notification (used
for a debugging client).\n\n* tools/server-side/svnpubsub/svnpubsub/
client.py\n (Notifications): Add __repr__\n\nSuggested by:
jun66j5', 'repository': '13f79535-47bb-0310-9956-ffa450edef68',
'format': 1, 'changed': {'subversion/trunk/tools/server-side/
svnpubsub/svnpubsub/client.py': {'flags': 'U '}}, 'date':
'2024-12-22 20:23:53 +0000 (Sun, 22 Dec 2024)', 'type': 'svn', 'id':
1922639})
event on 'http://svn.apache.org:2069/commits/ <http://
svn.apache.org:2069/commits/>': 'ping'
1734899044.039507
]]]
(The above is the client catching the commit of the __repr__ function).
I'll continue with asyncio shortly.
Cheers,
Daniel
