parkpow / deep-license-plate-recognition

Automatic License Plate Recognition (ALPR) or Automatic Number Plate Recognition (ANPR) software that works with any camera.
https://platerecognizer.com/
MIT License
523 stars 122 forks source link

feat: webhook middleware #212

Closed AashishDhakal closed 1 month ago

github-actions[bot] commented 1 month ago

Risk Level 3 - /home/runner/work/deep-license-plate-recognition/deep-license-plate-recognition/webhook-middleware/middlewares/soap.py

The code should handle potential exceptions from the SOAP client call to client.service.PostImage(**request_data) to prevent runtime errors. Additionally, the code should validate the json_data structure to ensure that the required keys are present before accessing them.


Risk Level 3 - /home/runner/work/deep-license-plate-recognition/deep-license-plate-recognition/webhook-middleware/consumer.py

The code should handle exceptions for json.loads to avoid potential crashes due to malformed JSON. Additionally, the use of os.getenv without a default value could lead to None being used in operations, which may cause TypeError. It's recommended to provide default values or handle None cases.


Risk Level 4 - /home/runner/work/deep-license-plate-recognition/deep-license-plate-recognition/webhook-middleware/middlewares/nx.py

The Timer object in session_create and parkpow_get_tags functions could lead to a memory leak if not properly managed. Consider using a more robust scheduling mechanism like sched.scheduler or an external task scheduler. Also, the use of sys.exit(1) within the exception handling could abruptly terminate the program, which might not be desirable in a middleware context.


šŸ”’šŸšŸ› ļø


Powered by Code Review GPT

AashishDhakal commented 1 month ago

@marcbelmont I am almost done. I made the common webhook consumer based on the synology code, but I see json key being indexed, https://github.com/parkpow/deep-license-plate-recognition/blob/master/webhooks/Synology/middleware_webhook_rest.py#L80

Is this outdated or we actually have that key on our webhooks? I didn't see that key on the webhook.

marcbelmont commented 1 month ago

@AashishDhakal here's the doc about the json field. https://guides.platerecognizer.com/docs/stream/results#vehicle-information

AashishDhakal commented 1 month ago

@marcbelmont It should be ready for review then.

ttayson commented 1 month ago

Everything seems correct and intuitive in how we are handling the different integrations. I would like a little more time to test the integrations, especially the main ones.

@marcbelmont Regarding the guide: will we have this Docker image directly from the Platerecognizer repository?

As for the other integration procedures (ex: Stream Config.ini), are we going to centralize them in another location, or will we transfer them to our guide? Similar to: https://guides.platerecognizer.com/docs/parkpow/integrations

marcbelmont commented 1 month ago

@ttayson @AashishDhakal Yes, the image will be hosted on Docker Hub. What's a good name for the image?
The documentation and examples should be on Guides. That is more user-friendly than GitHub, which may scare some people off.

ttayson commented 1 month ago

@ttayson @AashishDhakal Yes, the image will be hosted on Docker Hub. What's a good name for the image? The documentation and examples should be on Guides. That is more user-friendly than GitHub, which may scare some people off.

As an alternative and for greater clarity, I suggest text that includes: integrations, Addons, stream-gateway, or stream-integration

I will add an "integrations" category to the stream's sidebar in the guide and transcribe what's necessary.

ttayson commented 1 month ago

@AashishDhakal @marcbelmont, Iā€™m testing some integrations and noticed a limitation in the NX integration. The bookmarks are being created normally, but the import of Parkpow tags, if the user uses them, is not being performed. The function parkpow_get_tags() is not being triggered.

It is possible to work around this issue. In the previous code, it was called in the main function, but now it would be costly to call this function with every detection.

AashishDhakal commented 1 month ago
  • Support for Synology/ is missing

@marcbelmont That's rest protocol.

marcbelmont commented 1 month ago

@AashishDhakal synology_rest.py would be a better name. The forwarded data is quite specific.

AashishDhakal commented 1 month ago

@AashishDhakal @marcbelmont, Iā€™m testing some integrations and noticed a limitation in the NX integration. The bookmarks are being created normally, but the import of Parkpow tags, if the user uses them, is not being performed. The function parkpow_get_tags() is not being triggered.

It is possible to work around this issue. In the previous code, it was called in the main function, but now it would be costly to call this function with every detection.

@ttayson Can you please test it again? I have updated the code.

marcbelmont commented 1 month ago

@ttayson Can you confirm if everything works? @AashishDhakal For the name on Docker Hub, I'll go with platerecognizer/stream-gateway.

ttayson commented 1 month ago

@marcbelmont @AashishDhakal I tested the NX function again, and everything is okay. I also tested the crop_plate function, but I only got a cropped image when image_type = original. When image_type = vehicle, the cropped image doesn't work. I believe this is a limitation of the stream, which continues to send the plate box in its original size.

I can't test the other protocols directly, but they are simpler codes, just for conversion. It seems okay.