Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forward AXIS alpr camera events to ParkPow visits #207

Merged
merged 4 commits into from
Jun 6, 2024
Merged

Conversation

danleyb2
Copy link
Collaborator

@danleyb2 danleyb2 commented Jun 4, 2024

Not testing, lacking a camera

Copy link

github-actions bot commented Jun 4, 2024

Risk Level 2 - /home/runner/work/deep-license-plate-recognition/deep-license-plate-recognition/parkpow/axis-lpr/app.py

The code should handle potential environment variable issues more gracefully. If TOKEN or PP_URL are not set, the application will continue to run and will fail only when trying to use these variables. It's better to check for their existence at startup and fail early if they are not set. Additionally, the exception handling in process_files is too broad, which can mask different types of exceptions. It's recommended to catch specific exceptions or at least log the exception type.

Example:

if not token or not pp_url:
    raise ValueError('TOKEN and PP_URL environment variables are required')

# In process_files
except SpecificException as e:
    lgr.error('Specific error occurred', exc_info=e)
    return f'Error uploading files: {e}', 400

Risk Level 3 - /home/runner/work/deep-license-plate-recognition/deep-license-plate-recognition/parkpow/axis-lpr/parkpow.py

The log_vehicle method has a retry loop that could potentially raise a generic Exception after 5 failed attempts due to rate limiting, which is not informative for the caller. It would be better to define a custom exception class for clarity. Additionally, the retry loop does not handle other HTTP error codes besides 429, which could lead to unexpected behavior.

Example:

class ParkPowApiError(Exception):
    pass

# In log_vehicle
if response.status_code == 429:
    # existing retry logic
else:
    raise ParkPowApiError(f'HTTP error {response.status_code}: {response.text}')

🔒🐛🚦


Powered by Code Review GPT

Copy link
Collaborator

@marcbelmont marcbelmont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add unit test for process_files.

@danleyb2 danleyb2 requested a review from marcbelmont June 5, 2024 19:29
@marcbelmont marcbelmont merged commit fae4d10 into master Jun 6, 2024
1 check passed
@marcbelmont marcbelmont deleted the parkpow-axis branch June 6, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants