darvid / python-hyperscan

🐍 A CPython extension for the Hyperscan regular expression matching library.
https://python-hyperscan.readthedocs.io/en/latest/
MIT License
165 stars 28 forks source link

Callback event when HS is done with scanning #5

Closed Ivaylo-Korakov closed 5 years ago

Ivaylo-Korakov commented 5 years ago

HI, I am using your wrapper and it is great. But unfortunately, I am missing a feature that I really need for my project. I need to know when hyperscan is done with scanning the block of text that I provided. At the moment the wrapper only calls when a match is found but what if no match is found at all. I have no way of telling is it still searching or is it done just that it didn't find anything. I came up with a small fix and I am going to create a pull request. Keep in mind I don't know any C and all of this was done with google :smile:

darvid commented 5 years ago

hm, this sounds reasonable and the code looks good to me, but I'm having trouble understanding how a callback on success solves your use case for knowing when no match is found at all.

I could be wrong or misinterpreting, but I'm not sure if a callback is necessary considering you could wrap Database.scan in an exception handler, and if no exception is thrown, you can assume the scan completed with success. e.g.

try:
  db.scan(b'foo', match_event_handler)
except hyperscan.error:
  ...  # failure
else:
  ...  # success

since the GIL is acquired when the match handler is invoked, I don't think you need to worry about scan returning before any/all callbacks finish.

but even assuming you have a callback on scan completion, I'm not sure how you can infer no match found at all unless you use some external state (which is a good use case for context IMO).

anyway, having an explicit success callback seems like a reasonable feature to me, albeit I generally try to avoid adding extraneous features to this wrapper and keep it as close to a 1:1 API with Hyperscan as possible. if you can clarify the use case for me, that'd be great, and I'll be happy to merge. :+1:

Ivaylo-Korakov commented 5 years ago

Hi, I am really sorry for the long time that I took to respond. I am sorry to be honest I didn't know what I was doing :smile: You can close this issue and decline the PR. This behaviour can be implemented in Python instead of the C package. But if you like the feature you can leave it in I will be using it directly if it is implemented.