generator: NeMoGuardrails server support#1675
generator: NeMoGuardrails server support#1675jmartin-tech wants to merge 5 commits intoNVIDIA:mainfrom
Conversation
Signed-off-by: Jeffrey Martin <jemartin@nvidia.com>
Signed-off-by: Jeffrey Martin <jemartin@nvidia.com>
Signed-off-by: Jeffrey Martin <jemartin@nvidia.com>
6f27809 to
20fb006
Compare
leondz
left a comment
There was a problem hiding this comment.
couple of requests for clarification
| self.extra_body = {"guardrails": guardrails} | ||
|
|
||
|
|
||
| DEFAULT_CLASS = "NeMoGuardrails" |
There was a problem hiding this comment.
Is this still a sensible default?
There was a problem hiding this comment.
I would be in favor of shifting if there is consensus. That would elevate this to a breaking change for this addition as configurations that did not specify -t guardrails.NeMoGuardrails would change behavior.
| } | ||
|
|
||
| def __init__(self, name="", config_root=_config): | ||
| self.api_key = "not-used" # suppress any api_key from being sent as the server does not utilize one |
There was a problem hiding this comment.
is it worth setting ENV_VAR/key_env_var to None, or deleting ENV_VAR, to reduce potential confusion from this attr being inherited?
There was a problem hiding this comment.
Interesting point, currently NeMo Guardrails server does not offer any built in authentication, leading to the suppression added here is matched to the public docs from the sdk. I agree there is some confusion created by keeping the ENV_VAR on the class. I will test other patterns for this suppression.
There was a problem hiding this comment.
ENV_VAR is inherited so I added support for it being None as a suppression of the requirement in Configurable.
A place holder value still needs to be set here to ensure the openai client library does not raise when no key is supplied or found in their standard variable locations. Should we adjust this to apply the place holder after the call to super() to allow a user to provide api_key or key_env_var explicitly in the config file for this generator? This might future proof this better if NeMo Guardrails were to ever add support for injecting an authentication layer on the the hosting server.
| if hasattr(self, "extra_body") and self.extra_body and self.config_ids: | ||
| self.extra_body["guardrails"] = guardrails | ||
| else: | ||
| self.extra_body = {"guardrails": guardrails} |
There was a problem hiding this comment.
this is mysterious to me. i accept it, and welcome the mystery.
-- is something being worked around here? maybe a brief explanation in the docstring / a comment
There was a problem hiding this comment.
Official docs this was based on here. Will work out how to document this better on the class.
There was a problem hiding this comment.
Please validate update in d531028 meets the ask here.
Signed-off-by: Jeffrey Martin <jemartin@nvidia.com>
Signed-off-by: Jeffrey Martin <jemartin@nvidia.com>
Add support for new
OpenAICompatibleservice exposed via NVIDIA-NeMo/GuardrailsVerification
List the steps needed to make sure this thing works
garak -t guardrails.NeMoGuardrailsServer -n meta/llama-3.3-70b-instruct -p lmrcVerify the target guardrails configuration responds
start a NeMoGuardrails service example used is abc bot with a revised backing model without a default config_id:
garak --config config.guardrails.yaml -p lmrcabcconfiguration is activated and responds