Skip to content

Conversation

@Pouyanpi
Copy link
Collaborator

@Pouyanpi Pouyanpi commented Apr 29, 2025

Description

implement lazy loading and improve error handling for yara dependency

  • Implement lazy loading of yara module with proper type hints
  • Add error messages when yara is not available
  • Add helper function _check_yara_available() for consistent error handling

Related PR

#1091

Steps to Repro

# use a clean env

poetry env remove --all

# do not install `dev` dependencies 
poetry install -E "openai"

poetry run nemoguardrails chat --config="./examples/bots/hello_world"

expected output

Failed to register actions.py from nemoguardrails/library/injection_detection/actions.py in action dispatcher due to exception: No module named 'yara'
Starting the chat (Press Ctrl + C twice to quit) ...
Failed to register actions.py from nemoguardrails/library/injection_detection/actions.py in action dispatcher due to exception: No module named 'yara'

@Pouyanpi Pouyanpi added this to the v0.14.0 milestone Apr 29, 2025
@Pouyanpi Pouyanpi self-assigned this Apr 29, 2025
@Pouyanpi Pouyanpi marked this pull request as ready for review April 29, 2025 19:33
@Pouyanpi Pouyanpi added bug Something isn't working enhancement New feature or request refactoring and removed enhancement New feature or request refactoring labels May 1, 2025
Copy link
Collaborator

@erickgalinkin erickgalinkin left a comment

Choose a reason for hiding this comment

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

Looks good to me. Great catch!

@Pouyanpi Pouyanpi force-pushed the fix/injection-detection-dependency branch from cc621b5 to af3e53d Compare May 2, 2025 07:26
@codecov-commenter
Copy link

codecov-commenter commented May 2, 2025

Codecov Report

Attention: Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.00%. Comparing base (eede896) to head (d1b7509).
Report is 52 commits behind head on develop.

Files with missing lines Patch % Lines
...oguardrails/library/injection_detection/actions.py 82.35% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1162   +/-   ##
========================================
  Coverage    68.00%   68.00%           
========================================
  Files          161      161           
  Lines        15793    15801    +8     
========================================
+ Hits         10740    10746    +6     
- Misses        5053     5055    +2     
Flag Coverage Δ
python 68.00% <82.35%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...oguardrails/library/injection_detection/actions.py 78.18% <82.35%> (-0.25%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@trebedea trebedea left a comment

Choose a reason for hiding this comment

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

Looks good.

Only one suggestion, if this makes sense.
Would it be better to only have the action public and all the other functions private (seen as helpers, as they should not be used on their own - or can they be used separately)? This way we can check for availability of yara package only in the injection_detection action.

@Pouyanpi Pouyanpi force-pushed the fix/injection-detection-dependency branch from 39af000 to af3e53d Compare May 2, 2025 08:49
@Pouyanpi
Copy link
Collaborator Author

Pouyanpi commented May 2, 2025

Looks good.

Only one suggestion, if this makes sense. Would it be better to only have the action public and all the other functions private (seen as helpers, as they should not be used on their own - or can they be used separately)? This way we can check for availability of yara package only in the injection_detection action.

Totally, very nice suggestion, thank you! And at some point we are going to strictly define our public interfaces 👍🏻

Pouyanpi added 3 commits May 2, 2025 11:23
…ndency

- Implement lazy loading of yara module with proper type hints
- Add error messages when yara is not available
- Add helper function _check_yara_available() for consistent error handling
@Pouyanpi Pouyanpi force-pushed the fix/injection-detection-dependency branch from f99aee7 to d1b7509 Compare May 2, 2025 09:23
@Pouyanpi Pouyanpi merged commit 6d55b82 into develop May 2, 2025
17 checks passed
@Pouyanpi Pouyanpi deleted the fix/injection-detection-dependency branch May 2, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants