Skip to content

Fix call to LLM logger breaking scan#1888

Merged
kevinmessiaen merged 5 commits intomainfrom
task/fix-call-llm-logger
Apr 17, 2024
Merged

Fix call to LLM logger breaking scan#1888
kevinmessiaen merged 5 commits intomainfrom
task/fix-call-llm-logger

Conversation

@mattbit
Copy link
Member

@mattbit mattbit commented Apr 12, 2024

For scans that do not require the LLM client (special char injections, etc.), the scanner must not break if the LLM client is not set.

@sentry
Copy link

sentry bot commented Apr 12, 2024

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: giskard/scanner/scanner.py

Function Unhandled Issue
analyze OpenAIError: The api_key client option must be set either by passing api_key to the client or by setting the O... ...
Event Count: 3
analyze ModuleNotFoundError: No module named 'langchain.chains.LLMChain' __ma...
Event Count: 1
analyze RuntimeError: No issue detectors available. Scan will not be performed. ...
Event Count: 1
analyze LLMImportError: It seems that you are using Giskard LLM functionality but you are missing some required package. ... ...
Event Count: 1

Did you find this useful? React with a 👍 or 👎

@mattbit mattbit requested a review from kevinmessiaen April 12, 2024 08:20
llm_logger = _maybe_get_llm_logger()
if llm_logger is None:
return None
llm_logger = get_default_client().logger
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove this line, the llm_logger will already contain this value

@kevinmessiaen kevinmessiaen enabled auto-merge April 16, 2024 07:43
@kevinmessiaen kevinmessiaen dismissed andreybavt’s stale review April 16, 2024 07:43

Change has been applied

@kevinmessiaen kevinmessiaen disabled auto-merge April 16, 2024 07:43
@kevinmessiaen kevinmessiaen enabled auto-merge April 16, 2024 07:44
@sonarqubecloud
Copy link

@kevinmessiaen kevinmessiaen merged commit 294982c into main Apr 17, 2024
@kevinmessiaen kevinmessiaen deleted the task/fix-call-llm-logger branch April 17, 2024 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants