From eae3405bfc15e0dfdad92bad0167c742e3027ac3 Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Sat, 1 Apr 2023 19:00:59 -0500 Subject: [PATCH 01/10] Starting here: remove verify class --- python/semantic_kernel/diagnostics/verify.py | 105 ------------------- 1 file changed, 105 deletions(-) delete mode 100644 python/semantic_kernel/diagnostics/verify.py diff --git a/python/semantic_kernel/diagnostics/verify.py b/python/semantic_kernel/diagnostics/verify.py deleted file mode 100644 index 219a535e1afb..000000000000 --- a/python/semantic_kernel/diagnostics/verify.py +++ /dev/null @@ -1,105 +0,0 @@ -# Copyright (c) Microsoft. All rights reserved. - -import os -import re -from typing import Any, Optional - -from semantic_kernel.diagnostics.validation_exception import ValidationException -from semantic_kernel.kernel_exception import KernelException - - -class Verify: - @staticmethod - def not_null(value: Optional[Any], message: str) -> None: - if value is not None: - return - - raise ValidationException(ValidationException.ErrorCodes.NullValue, message) - - @staticmethod - def not_empty(value: Optional[str], message: str) -> None: - Verify.not_null(value, message) - if value.strip() != "": # type: ignore - return - - raise ValidationException(ValidationException.ErrorCodes.EmptyValue, message) - - @staticmethod - def valid_skill_name(value: Optional[str]) -> None: - Verify.not_empty(value, "The skill name cannot be empty") - - SKILL_NAME_REGEX = r"^[0-9A-Za-z_]*$" - if re.match(SKILL_NAME_REGEX, value): # type: ignore - return - - raise KernelException( - KernelException.ErrorCodes.InvalidFunctionDescription, - "A skill name can contain only latin letters, digits 0-9, " - f"and underscores: '{value}' is not a valid skill name.", - ) - - @staticmethod - def valid_function_name(value: Optional[str]) -> None: - Verify.not_empty(value, "The function name cannot be empty") - - FUNCTION_NAME_REGEX = r"^[0-9A-Za-z_]*$" - if re.match(FUNCTION_NAME_REGEX, value): # type: ignore - return - - raise KernelException( - KernelException.ErrorCodes.InvalidFunctionDescription, - "A function name can contain only latin letters, digits 0-9, " - f"and underscores: '{value}' is not a valid function name.", - ) - - @staticmethod - def valid_function_param_name(value: Optional[str]) -> None: - Verify.not_empty(value, "The function parameter name cannot be empty") - - FUNCTION_PARAM_NAME_REGEX = r"^[0-9A-Za-z_]*$" - if re.match(FUNCTION_PARAM_NAME_REGEX, value): # type: ignore - return - - raise KernelException( - KernelException.ErrorCodes.InvalidFunctionDescription, - "A function parameter name can contain only latin letters, " - f"digits 0-9, and underscores: '{value}' is not a valid " - f"function parameter name.", - ) - - @staticmethod - def starts_with(text: str, prefix: Optional[str], message: str) -> None: - Verify.not_empty(text, "The text to verify cannot be empty") - Verify.not_null(prefix, "The prefix to verify cannot be null") - - if text.startswith(prefix): # type: ignore - return - - raise ValidationException(ValidationException.ErrorCodes.MissingPrefix, message) - - @staticmethod - def directory_exists(path: str): - Verify.not_empty(path, "The path to verify cannot be empty") - - if os.path.isdir(path): - return - - raise ValidationException( - ValidationException.ErrorCodes.DirectoryNotFound, - f"Directory not found: '{path}'", - ) - - @staticmethod - def parameters_unique(parameters: list): # TODO: ParameterView - name_set = set() - - for parameter in parameters: - if parameter.name in name_set: - raise KernelException( - KernelException.ErrorCodes.InvalidFunctionDescription, - "The function has two or more parameters " - f"with the same name '{parameter.name}'", - ) - - Verify.not_empty(parameter.name, "The function parameter name is empty") - name_set.add(parameter.name.lower()) From 81fb865436c7c121a16b8250459330cc27b2edff Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Mon, 3 Apr 2023 11:21:31 -0500 Subject: [PATCH 02/10] Work in progress: removing Verify --- .../open_ai/services/azure_chat_completion.py | 15 ++-- .../open_ai/services/azure_open_ai_config.py | 16 ++--- .../open_ai/services/azure_text_completion.py | 15 ++-- .../open_ai/services/azure_text_embedding.py | 15 ++-- .../services/open_ai_chat_completion.py | 4 +- .../ai/open_ai/services/open_ai_config.py | 8 +-- .../services/open_ai_text_completion.py | 7 +- .../configuration/kernel_config.py | 19 +++-- .../core_skills/text_memory_skill.py | 31 ++++---- .../semantic_kernel/diagnostics/validation.py | 70 +++++++++++++++++++ python/semantic_kernel/kernel.py | 26 ++++--- python/semantic_kernel/kernel_builder.py | 13 ++-- .../import_semantic_skill_from_directory.py | 13 +++- .../inline_function_definitions.py | 14 +++- .../kernel_extensions/memory_configuration.py | 16 ++--- .../orchestration/context_variables.py | 8 +-- .../orchestration/sk_context.py | 4 +- .../orchestration/sk_function.py | 25 ++++--- .../orchestration/sk_function_base.py | 4 ++ .../skill_definition/function_view.py | 15 +++- .../skill_definition/parameter_view.py | 16 ++++- .../skill_definition/skill_collection.py | 11 +-- 22 files changed, 250 insertions(+), 115 deletions(-) create mode 100644 python/semantic_kernel/diagnostics/validation.py diff --git a/python/semantic_kernel/ai/open_ai/services/azure_chat_completion.py b/python/semantic_kernel/ai/open_ai/services/azure_chat_completion.py index bbd8d0812351..eb8d395da24a 100644 --- a/python/semantic_kernel/ai/open_ai/services/azure_chat_completion.py +++ b/python/semantic_kernel/ai/open_ai/services/azure_chat_completion.py @@ -7,7 +7,6 @@ from semantic_kernel.ai.open_ai.services.open_ai_chat_completion import ( OpenAIChatCompletion, ) -from semantic_kernel.diagnostics.verify import Verify class AzureChatCompletion(OpenAIChatCompletion): @@ -22,12 +21,14 @@ def __init__( api_version: str, logger: Optional[Logger] = None, ) -> None: - Verify.not_empty(deployment_name, "You must provide a deployment name") - Verify.not_empty(api_key, "The Azure API key cannot be empty") - Verify.not_empty(endpoint, "The Azure endpoint cannot be empty") - Verify.starts_with( - endpoint, "https://", "The Azure endpoint must start with https://" - ) + if not deployment_name: + raise ValueError("The deployment name cannot be empty") + if not api_key: + raise ValueError("The Azure API key cannot be empty") + if not endpoint: + raise ValueError("The Azure endpoint cannot be empty") + if not endpoint.startswith("https://"): + raise ValueError("The Azure endpoint must start with https://") self._endpoint = endpoint self._api_version = api_version diff --git a/python/semantic_kernel/ai/open_ai/services/azure_open_ai_config.py b/python/semantic_kernel/ai/open_ai/services/azure_open_ai_config.py index b7ece681f330..bd560a432c22 100644 --- a/python/semantic_kernel/ai/open_ai/services/azure_open_ai_config.py +++ b/python/semantic_kernel/ai/open_ai/services/azure_open_ai_config.py @@ -1,7 +1,5 @@ # Copyright (c) Microsoft. All rights reserved. -from semantic_kernel.diagnostics.verify import Verify - # TODO: support for AAD auth. class AzureOpenAIConfig: @@ -41,12 +39,14 @@ def __init__( api_version {str} -- Azure OpenAI API version, https://learn.microsoft.com/azure/cognitive-services/openai/reference """ - Verify.not_empty(deployment_name, "The deployment name is empty") - Verify.not_empty(endpoint, "The endpoint is empty") - Verify.starts_with( - endpoint, "https://", "The endpoint URL must start with https://" - ) - Verify.not_empty(api_key, "The API key is empty") + if not deployment_name: + raise ValueError("The deployment name cannot be empty") + if not endpoint: + raise ValueError("The endpoint cannot be empty") + if not endpoint.startswith("https://"): + raise ValueError("The endpoint must start with https://") + if not api_key: + raise ValueError("The API key cannot be empty") self.deployment_name = deployment_name self.endpoint = endpoint diff --git a/python/semantic_kernel/ai/open_ai/services/azure_text_completion.py b/python/semantic_kernel/ai/open_ai/services/azure_text_completion.py index ffba59a8559e..f0431e7a5d9a 100644 --- a/python/semantic_kernel/ai/open_ai/services/azure_text_completion.py +++ b/python/semantic_kernel/ai/open_ai/services/azure_text_completion.py @@ -7,7 +7,6 @@ from semantic_kernel.ai.open_ai.services.open_ai_text_completion import ( OpenAITextCompletion, ) -from semantic_kernel.diagnostics.verify import Verify class AzureTextCompletion(OpenAITextCompletion): @@ -22,12 +21,14 @@ def __init__( api_version: str, logger: Optional[Logger] = None, ) -> None: - Verify.not_empty(deployment_name, "You must provide a deployment name") - Verify.not_empty(api_key, "The Azure API key cannot be empty") - Verify.not_empty(endpoint, "The Azure endpoint cannot be empty") - Verify.starts_with( - endpoint, "https://", "The Azure endpoint must start with https://" - ) + if not deployment_name: + raise ValueError("The deployment name cannot be empty") + if not api_key: + raise ValueError("The Azure API key cannot be empty") + if not endpoint: + raise ValueError("The Azure endpoint cannot be empty") + if not endpoint.startswith("https://"): + raise ValueError("The Azure endpoint must start with https://") self._endpoint = endpoint self._api_version = api_version diff --git a/python/semantic_kernel/ai/open_ai/services/azure_text_embedding.py b/python/semantic_kernel/ai/open_ai/services/azure_text_embedding.py index dc610401c55d..a5728a48a1ad 100644 --- a/python/semantic_kernel/ai/open_ai/services/azure_text_embedding.py +++ b/python/semantic_kernel/ai/open_ai/services/azure_text_embedding.py @@ -7,7 +7,6 @@ from semantic_kernel.ai.open_ai.services.open_ai_text_embedding import ( OpenAITextEmbedding, ) -from semantic_kernel.diagnostics.verify import Verify class AzureTextEmbedding(OpenAITextEmbedding): @@ -22,12 +21,14 @@ def __init__( api_version: str, logger: Optional[Logger] = None, ) -> None: - Verify.not_empty(deployment_name, "You must provide a deployment name") - Verify.not_empty(api_key, "The Azure API key cannot be empty") - Verify.not_empty(endpoint, "The Azure endpoint cannot be empty") - Verify.starts_with( - endpoint, "https://", "The Azure endpoint must start with https://" - ) + if not deployment_name: + raise ValueError("The deployment name cannot be empty") + if not api_key: + raise ValueError("The Azure API key cannot be empty") + if not endpoint: + raise ValueError("The Azure endpoint cannot be empty") + if not endpoint.startswith("https://"): + raise ValueError("The Azure endpoint must start with https://") self._endpoint = endpoint self._api_version = api_version diff --git a/python/semantic_kernel/ai/open_ai/services/open_ai_chat_completion.py b/python/semantic_kernel/ai/open_ai/services/open_ai_chat_completion.py index 75ab0014cf07..61bf39b30c00 100644 --- a/python/semantic_kernel/ai/open_ai/services/open_ai_chat_completion.py +++ b/python/semantic_kernel/ai/open_ai/services/open_ai_chat_completion.py @@ -6,7 +6,6 @@ from semantic_kernel.ai.ai_exception import AIException from semantic_kernel.ai.chat_completion_client_base import ChatCompletionClientBase from semantic_kernel.ai.chat_request_settings import ChatRequestSettings -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.utils.null_logger import NullLogger @@ -65,7 +64,8 @@ async def complete_chat_async( Returns: str -- The completed text. """ - Verify.not_null(request_settings, "The request settings cannot be empty") + if request_settings is None: + raise ValueError("The request settings cannot be empty") if request_settings.max_tokens < 1: raise AIException( diff --git a/python/semantic_kernel/ai/open_ai/services/open_ai_config.py b/python/semantic_kernel/ai/open_ai/services/open_ai_config.py index 6e1dfe20c8e3..dc7a4dfb1e47 100644 --- a/python/semantic_kernel/ai/open_ai/services/open_ai_config.py +++ b/python/semantic_kernel/ai/open_ai/services/open_ai_config.py @@ -2,8 +2,6 @@ from typing import Optional -from semantic_kernel.diagnostics.verify import Verify - # TODO: allow for overriding endpoints class OpenAIConfig: @@ -33,8 +31,10 @@ def __init__( This is usually optional unless your account belongs to multiple organizations. """ - Verify.not_empty(model_id, "The model ID is empty") - Verify.not_empty(api_key, "The API key is empty") + if not model_id: + raise ValueError("The model ID cannot be empty") + if not api_key: + raise ValueError("The API key cannot be empty") self.model_id = model_id self.api_key = api_key diff --git a/python/semantic_kernel/ai/open_ai/services/open_ai_text_completion.py b/python/semantic_kernel/ai/open_ai/services/open_ai_text_completion.py index f7eb3a942916..b3b4db676e5d 100644 --- a/python/semantic_kernel/ai/open_ai/services/open_ai_text_completion.py +++ b/python/semantic_kernel/ai/open_ai/services/open_ai_text_completion.py @@ -6,7 +6,6 @@ from semantic_kernel.ai.ai_exception import AIException from semantic_kernel.ai.complete_request_settings import CompleteRequestSettings from semantic_kernel.ai.text_completion_client_base import TextCompletionClientBase -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.utils.null_logger import NullLogger @@ -65,8 +64,10 @@ async def complete_simple_async( Returns: str -- The completed text. """ - Verify.not_empty(prompt, "The prompt is empty") - Verify.not_null(request_settings, "The request settings cannot be empty") + if not prompt: + raise ValueError("The prompt cannot be empty") + if request_settings is None: + raise ValueError("The request settings cannot be empty") if request_settings.max_tokens < 1: raise AIException( diff --git a/python/semantic_kernel/configuration/kernel_config.py b/python/semantic_kernel/configuration/kernel_config.py index 32488646ee99..ce262aca18fa 100644 --- a/python/semantic_kernel/configuration/kernel_config.py +++ b/python/semantic_kernel/configuration/kernel_config.py @@ -6,7 +6,6 @@ from semantic_kernel.ai.open_ai.services.open_ai_config import OpenAIConfig from semantic_kernel.configuration.backend_config import BackendConfig from semantic_kernel.configuration.backend_types import BackendType -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_exception import KernelException from semantic_kernel.reliability.pass_through_without_retry import ( PassThroughWithoutRetry, @@ -31,7 +30,8 @@ def add_openai_chat_backend( org_id: Optional[str] = None, overwrite: bool = False, ) -> "KernelConfig": - Verify.not_empty(name, "The backend name is empty") + if not name: + raise ValueError("The backend name cannot be empty") if not overwrite and name in self._chat_backends: raise KernelException( @@ -58,7 +58,8 @@ def add_azure_openai_chat_backend( api_version: str = "2022-12-01", overwrite: bool = False, ) -> "KernelConfig": - Verify.not_empty(name, "The backend name is empty") + if not name: + raise ValueError("The backend name cannot be empty") if not overwrite and name in self._chat_backends: raise KernelException( @@ -87,7 +88,8 @@ def add_azure_openai_completion_backend( api_version: str = "2022-12-01", overwrite: bool = False, ) -> "KernelConfig": - Verify.not_empty(name, "The backend name is empty") + if not name: + raise ValueError("The backend name cannot be empty") if not overwrite and name in self._completion_backends: raise KernelException( @@ -115,7 +117,8 @@ def add_openai_completion_backend( org_id: Optional[str] = None, overwrite: bool = False, ) -> "KernelConfig": - Verify.not_empty(name, "The backend name is empty") + if not name: + raise ValueError("The backend name cannot be empty") if not overwrite and name in self._completion_backends: raise KernelException( @@ -142,7 +145,8 @@ def add_azure_open_ai_embeddings_backend( api_version: str = "2022-12-01", overwrite: bool = False, ) -> "KernelConfig": - Verify.not_empty(name, "The backend name is empty") + if not name: + raise ValueError("The backend name cannot be empty") if not overwrite and name in self._embeddings_backends: raise KernelException( @@ -170,7 +174,8 @@ def add_open_ai_embeddings_backend( org_id: Optional[str] = None, overwrite: bool = False, ) -> "KernelConfig": - Verify.not_empty(name, "The backend name is empty") + if not name: + raise ValueError("The backend name cannot be empty") if not overwrite and name in self._embeddings_backends: raise KernelException( diff --git a/python/semantic_kernel/core_skills/text_memory_skill.py b/python/semantic_kernel/core_skills/text_memory_skill.py index ca2e893c62b2..1c478d4fe37b 100644 --- a/python/semantic_kernel/core_skills/text_memory_skill.py +++ b/python/semantic_kernel/core_skills/text_memory_skill.py @@ -1,6 +1,5 @@ # Copyright (c) Microsoft. All rights reserved. -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.orchestration.sk_context import SKContext from semantic_kernel.skill_definition import sk_function, sk_function_context_parameter @@ -44,19 +43,18 @@ async def recall_async(self, ask: str, context: SKContext) -> str: Returns: The nearest item from the memory store """ - Verify.not_null(context.variables, "Context has no variables") - assert context.variables is not None # for type checker - Verify.not_null(context.memory, "Context has no memory") - assert context.memory is not None # for type checker + if context.variables is None: + raise ValueError("Context has no variables") + if context.memory is None: + raise ValueError("Context has no memory") collection = ( context.variables[TextMemorySkill.COLLECTION_PARAM] if context.variables.contains_key(TextMemorySkill.COLLECTION_PARAM) else TextMemorySkill.DEFAULT_COLLECTION ) - Verify.not_empty( - collection, "Memory collection not defined for TextMemorySkill" - ) + if not collection: + raise ValueError("Memory collection not defined for TextMemorySkill") relevance = ( context.variables[TextMemorySkill.RELEVANCE_PARAM] @@ -104,26 +102,25 @@ async def save_async(self, text: str, context: SKContext): context -- Contains the 'collection' to save the information and unique 'key' to associate with the information """ - Verify.not_null(context.variables, "Context has no variables") - assert context.variables is not None # for type checker - Verify.not_null(context.memory, "Context has no memory") - assert context.memory is not None # for type checker + if context.variables is None: + raise ValueError("Context has no variables") + if context.memory is None: + raise ValueError("Context has no memory") collection = ( context.variables[TextMemorySkill.COLLECTION_PARAM] if context.variables.contains_key(TextMemorySkill.COLLECTION_PARAM) else TextMemorySkill.DEFAULT_COLLECTION ) - Verify.not_empty( - collection, "Memory collection not defined for TextMemorySkill" - ) + if not collection: + raise ValueError("Memory collection not defined for TextMemorySkill") key = ( context.variables[TextMemorySkill.KEY_PARAM] if context.variables.contains_key(TextMemorySkill.KEY_PARAM) else None ) - Verify.not_empty(key, "Memory key not defined for TextMemorySkill") - assert key is not None # for type checker + if not key: + raise ValueError("Memory key not defined for TextMemorySkill") await context.memory.save_information_async(collection, text=text, id=key) diff --git a/python/semantic_kernel/diagnostics/validation.py b/python/semantic_kernel/diagnostics/validation.py new file mode 100644 index 000000000000..5dea044ad491 --- /dev/null +++ b/python/semantic_kernel/diagnostics/validation.py @@ -0,0 +1,70 @@ +# Copyright (c) Microsoft. All rights reserved. + +from re import match as re_match +from typing import Optional + + +def valid_skill_name(value: Optional[str]) -> None: + """ + Validates that the skill name is valid. + + Valid skill names are non-empty and + match the regex: [0-9A-Za-z_]* + + :param value: The skill name to validate. + + :raises ValueError: If the skill name is invalid. + """ + if not value: + raise ValueError("The skill name cannot be empty") + + SKILL_NAME_REGEX = r"^[0-9A-Za-z_]*$" + if not re_match(SKILL_NAME_REGEX, value): + raise ValueError( + f"Invalid skill name: {value}. Skill " + f"names must match the regex: {SKILL_NAME_REGEX}" + ) + + +def valid_function_name(value: Optional[str]) -> None: + """ + Validates that the function name is valid. + + Valid function names are non-empty and + match the regex: [0-9A-Za-z_]* + + :param value: The function name to validate. + + :raises ValueError: If the function name is invalid. + """ + if not value: + raise ValueError("The function name cannot be empty") + + FUNCTION_NAME_REGEX = r"^[0-9A-Za-z_]*$" + if not re_match(FUNCTION_NAME_REGEX, value): + raise ValueError( + f"Invalid function name: {value}. Function " + f"names must match the regex: {FUNCTION_NAME_REGEX}" + ) + + +def valid_function_param_name(value: Optional[str]) -> None: + """ + Validates that the function parameter name is valid. + + Valid function parameter names are non-empty and + match the regex: [0-9A-Za-z_]* + + :param value: The function parameter name to validate. + + :raises ValueError: If the function parameter name is invalid. + """ + if not value: + raise ValueError("The function parameter name cannot be empty") + + FUNCTION_PARAM_NAME_REGEX = r"^[0-9A-Za-z_]*$" + if not re_match(FUNCTION_PARAM_NAME_REGEX, value): + raise ValueError( + f"Invalid function parameter name: {value}. Function parameter " + f"names must match the regex: {FUNCTION_PARAM_NAME_REGEX}" + ) diff --git a/python/semantic_kernel/kernel.py b/python/semantic_kernel/kernel.py index d6c1c3a50c71..43a53f5708ce 100644 --- a/python/semantic_kernel/kernel.py +++ b/python/semantic_kernel/kernel.py @@ -18,7 +18,6 @@ ) from semantic_kernel.configuration.backend_types import BackendType from semantic_kernel.configuration.kernel_config import KernelConfig -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_base import KernelBase from semantic_kernel.kernel_exception import KernelException from semantic_kernel.memory.semantic_text_memory_base import SemanticTextMemoryBase @@ -90,8 +89,17 @@ def register_semantic_function( skill_name = SkillCollection.GLOBAL_SKILL assert skill_name is not None # for type checker - Verify.valid_skill_name(skill_name) - Verify.valid_function_name(function_name) + validate_skill_name(skill_name) + if not re_match(SKFunctionBase.SKILL_NAME_REGEX, skill_name): + raise ValueError( + f"Invalid skill name: {skill_name}. Skill names " + f"must match the regex: {SKFunctionBase.SKILL_NAME_REGEX}" + ) + if not re_match(SKFunctionBase.FUNCTION_NAME_REGEX, function_name): + raise ValueError( + f"Invalid function name: {function_name}. Function names " + f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" + ) function = self._create_semantic_function( skill_name, function_name, function_config @@ -256,7 +264,9 @@ def _create_semantic_function( "Azure OpenAI Chat backend is not implemented yet" ) elif backend.backend_type == BackendType.OpenAI: - Verify.not_null(backend.open_ai, "OpenAI configuration is missing") + if backend.open_ai is None: + raise ValueError("OpenAI configuration is missing") + function.set_chat_backend( lambda: OpenAIChatCompletion( backend.open_ai.model_id, # type: ignore @@ -287,9 +297,8 @@ def _create_semantic_function( ) if backend.backend_type == BackendType.AzureOpenAI: - Verify.not_null( - backend.azure_open_ai, "Azure OpenAI configuration is missing" - ) + if backend.azure_open_ai is None: + raise ValueError("Azure OpenAI configuration is missing") function.set_ai_backend( lambda: AzureTextCompletion( backend.azure_open_ai.deployment_name, # type: ignore @@ -300,7 +309,8 @@ def _create_semantic_function( ) ) elif backend.backend_type == BackendType.OpenAI: - Verify.not_null(backend.open_ai, "OpenAI configuration is missing") + if backend.open_ai is None: + raise ValueError("OpenAI configuration is missing") function.set_ai_backend( lambda: OpenAITextCompletion( backend.open_ai.model_id, # type: ignore diff --git a/python/semantic_kernel/kernel_builder.py b/python/semantic_kernel/kernel_builder.py index 37718217a19d..845b2e459521 100644 --- a/python/semantic_kernel/kernel_builder.py +++ b/python/semantic_kernel/kernel_builder.py @@ -4,7 +4,6 @@ from typing import Callable, Optional from semantic_kernel.configuration.kernel_config import KernelConfig -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel import Kernel from semantic_kernel.kernel_base import KernelBase from semantic_kernel.kernel_extensions import KernelExtensions @@ -31,22 +30,26 @@ def __init__( self._log = log def with_configuration(self, config: KernelConfig) -> "KernelBuilder": - Verify.not_null(config, "The configuration instance provided is None") + if config is None: + raise ValueError("The configuration instance provided is None") self._config = config return self def with_memory(self, memory: SemanticTextMemoryBase) -> "KernelBuilder": - Verify.not_null(memory, "The memory instance provided is None") + if memory is None: + raise ValueError("The memory instance provided is None") self._memory = memory return self def with_memory_storage(self, storage: MemoryStoreBase) -> "KernelBuilder": - Verify.not_null(storage, "The memory storage instance provided is None") + if storage is None: + raise ValueError("The memory storage instance provided is None") self._memory_storage = storage return self def with_logger(self, log: Logger) -> "KernelBuilder": - Verify.not_null(log, "The logger instance provided is None") + if log is None: + raise ValueError("The logger instance provided is None") self._log = log return self diff --git a/python/semantic_kernel/kernel_extensions/import_semantic_skill_from_directory.py b/python/semantic_kernel/kernel_extensions/import_semantic_skill_from_directory.py index dfd6d3c12cb1..73c331b94cb7 100644 --- a/python/semantic_kernel/kernel_extensions/import_semantic_skill_from_directory.py +++ b/python/semantic_kernel/kernel_extensions/import_semantic_skill_from_directory.py @@ -3,8 +3,8 @@ import glob import os from typing import Dict +from re import match as re_match -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_base import KernelBase from semantic_kernel.orchestration.sk_function_base import SKFunctionBase from semantic_kernel.semantic_functions.prompt_template import PromptTemplate @@ -22,10 +22,17 @@ def import_semantic_skill_from_directory( CONFIG_FILE = "config.json" PROMPT_FILE = "skprompt.txt" - Verify.valid_skill_name(skill_directory_name) + if not re_match(SKFunctionBase.SKILL_NAME_REGEX, skill_directory_name): + raise ValueError( + f"Invalid skill name: {skill_directory_name}. Skill names " + f"must match the regex: {SKFunctionBase.SKILL_NAME_REGEX}" + ) + skill_directory = os.path.join(parent_directory, skill_directory_name) skill_directory = os.path.abspath(skill_directory) - Verify.directory_exists(skill_directory) + + if not os.path.exists(skill_directory): + raise ValueError(f"Skill directory does not exist: {skill_directory}") skill = {} diff --git a/python/semantic_kernel/kernel_extensions/inline_function_definitions.py b/python/semantic_kernel/kernel_extensions/inline_function_definitions.py index d2bf6e3fa42a..074ef2507d25 100644 --- a/python/semantic_kernel/kernel_extensions/inline_function_definitions.py +++ b/python/semantic_kernel/kernel_extensions/inline_function_definitions.py @@ -2,8 +2,8 @@ from typing import TYPE_CHECKING, List, Optional from uuid import uuid4 +from re import match as re_match -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_base import KernelBase from semantic_kernel.semantic_functions.prompt_template import PromptTemplate from semantic_kernel.semantic_functions.prompt_template_config import ( @@ -53,9 +53,17 @@ def create_semantic_function( ), ) - Verify.valid_function_name(function_name) + if not re_match(SKFunctionBase.FUNCTION_NAME_REGEX, function_name): + raise ValueError( + f"Invalid function name: {function_name}. Function names " + f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" + ) if skill_name is not None: - Verify.valid_skill_name(skill_name) + if not re_match(SKFunctionBase.SKILL_NAME_REGEX, skill_name): + raise ValueError( + f"Invalid skill name: {skill_name}. Skill names " + f"must match the regex: {SKFunctionBase.SKILL_NAME_REGEX}" + ) template = PromptTemplate(prompt_template, kernel.prompt_template_engine, config) function_config = SemanticFunctionConfig(config, template) diff --git a/python/semantic_kernel/kernel_extensions/memory_configuration.py b/python/semantic_kernel/kernel_extensions/memory_configuration.py index 22b10de91333..fd058656d713 100644 --- a/python/semantic_kernel/kernel_extensions/memory_configuration.py +++ b/python/semantic_kernel/kernel_extensions/memory_configuration.py @@ -10,7 +10,6 @@ OpenAITextEmbedding, ) from semantic_kernel.configuration.backend_types import BackendType -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_base import KernelBase from semantic_kernel.memory.memory_store_base import MemoryStoreBase from semantic_kernel.memory.semantic_text_memory import SemanticTextMemory @@ -23,13 +22,12 @@ def use_memory( ) -> None: if embeddings_generator is None: backend_label = kernel.config.default_embeddings_backend - Verify.not_empty(backend_label, "The embedding backend label is empty") + if not backend_label: + raise ValueError("The embedding backend label is empty") embeddings_backend_config = kernel.config.get_embeddings_backend(backend_label) - Verify.not_null( - embeddings_backend_config, - f"AI configuration is missing for: {backend_label}", - ) + if embeddings_backend_config is None: + raise ValueError(f"AI configuration is missing for: {backend_label}") if embeddings_backend_config.backend_type == BackendType.OpenAI: assert embeddings_backend_config.open_ai is not None # for mypy @@ -54,7 +52,9 @@ def use_memory( "is not yet implemented" ) - Verify.not_null(storage, "The storage instance provided is None") - Verify.not_null(embeddings_generator, "The embedding generator is None") + if storage is None: + raise ValueError("The storage instance provided is None") + if embeddings_generator is None: + raise ValueError("The embedding generator is None") kernel.register_memory(SemanticTextMemory(storage, embeddings_generator)) diff --git a/python/semantic_kernel/orchestration/context_variables.py b/python/semantic_kernel/orchestration/context_variables.py index 35696066971d..bab33f414c61 100644 --- a/python/semantic_kernel/orchestration/context_variables.py +++ b/python/semantic_kernel/orchestration/context_variables.py @@ -3,8 +3,6 @@ from typing import Dict, Tuple -from semantic_kernel.diagnostics.verify import Verify - class ContextVariables: def __init__(self, content: str = "", variables: Dict[str, str] = None) -> None: @@ -36,7 +34,8 @@ def merge_or_overwrite( return self def set(self, name: str, value: str) -> "ContextVariables": - Verify.not_empty(name, "The variable name is empty") + if not name: + raise ValueError("The variable name cannot be empty") name = name.lower() if value is not None: @@ -58,7 +57,8 @@ def __getitem__(self, name: str) -> str: return self._variables[name] def __setitem__(self, name: str, value: str) -> None: - Verify.not_empty(name, "The variable name is empty") + if not name: + raise ValueError("The variable name cannot be empty") name = name.lower() self._variables[name] = value diff --git a/python/semantic_kernel/orchestration/sk_context.py b/python/semantic_kernel/orchestration/sk_context.py index 59de9c29f471..f2cc0eae2a6b 100644 --- a/python/semantic_kernel/orchestration/sk_context.py +++ b/python/semantic_kernel/orchestration/sk_context.py @@ -3,7 +3,6 @@ from logging import Logger from typing import Any, Literal, Optional, Tuple, Union -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_exception import KernelException from semantic_kernel.memory.semantic_text_memory_base import SemanticTextMemoryBase from semantic_kernel.orchestration.context_variables import ContextVariables @@ -177,7 +176,8 @@ def func(self, skill_name: str, function_name: str): Returns: SKFunctionBase -- The function. """ - Verify.not_null(self._skill_collection, "The skill collection hasn't been set") + if self._skill_collection is None: + raise ValueError("The skill collection hasn't been set") assert self._skill_collection is not None # for type checker if self._skill_collection.has_native_function(skill_name, function_name): diff --git a/python/semantic_kernel/orchestration/sk_function.py b/python/semantic_kernel/orchestration/sk_function.py index a39f1ffc48c2..ba0d978e53f5 100644 --- a/python/semantic_kernel/orchestration/sk_function.py +++ b/python/semantic_kernel/orchestration/sk_function.py @@ -8,7 +8,6 @@ from semantic_kernel.ai.chat_request_settings import ChatRequestSettings from semantic_kernel.ai.complete_request_settings import CompleteRequestSettings from semantic_kernel.ai.text_completion_client_base import TextCompletionClientBase -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_exception import KernelException from semantic_kernel.memory.null_memory import NullMemory from semantic_kernel.memory.semantic_text_memory_base import SemanticTextMemoryBase @@ -43,7 +42,8 @@ class SKFunction(SKFunctionBase): @staticmethod def from_native_method(method, skill_name="", log=None) -> "SKFunction": - Verify.not_null(method, "Method is empty") + if method is None: + raise ValueError("Method cannot be empty") assert method.__sk_function__ is not None, "Method is not a SK function" assert method.__sk_function_name__ is not None, "Method name is empty" @@ -88,10 +88,12 @@ def from_semantic_config( function_config: SemanticFunctionConfig, log: Optional[Logger] = None, ) -> "SKFunction": - Verify.not_null(function_config, "Function configuration is empty") + if function_config is None: + raise ValueError("Function configuration cannot be empty") async def _local_func(client, request_settings, context): - Verify.not_null(client, "AI LLM backend is empty") + if client is None: + raise ValueError("AI LLM backend cannot be empty") try: if function_config.has_chat_prompt: @@ -200,7 +202,8 @@ def set_default_skill_collection( def set_ai_backend( self, ai_backend: Callable[[], TextCompletionClientBase] ) -> "SKFunction": - Verify.not_null(ai_backend, "AI LLM backend factory is empty") + if ai_backend is None: + raise ValueError("AI LLM backend factory cannot be empty") self._verify_is_semantic() self._ai_backend = ai_backend() return self @@ -208,19 +211,22 @@ def set_ai_backend( def set_chat_backend( self, chat_backend: Callable[[], ChatCompletionClientBase] ) -> "SKFunction": - Verify.not_null(chat_backend, "Chat LLM backend factory is empty") + if chat_backend is None: + raise ValueError("Chat LLM backend factory cannot be empty") self._verify_is_semantic() self._chat_backend = chat_backend() return self def set_ai_configuration(self, settings: CompleteRequestSettings) -> "SKFunction": - Verify.not_null(settings, "AI LLM request settings are empty") + if settings is None: + raise ValueError("AI LLM request settings cannot be empty") self._verify_is_semantic() self._ai_request_settings = settings return self def set_chat_configuration(self, settings: ChatRequestSettings) -> "SKFunction": - Verify.not_null(settings, "Chat LLM request settings are empty") + if settings is None: + raise ValueError("Chat LLM request settings cannot be empty") self._verify_is_semantic() self._chat_request_settings = settings return self @@ -242,7 +248,8 @@ async def invoke_async( log: Optional[Logger] = None, ) -> SKContext: if context is None: - Verify.not_null(self._skill_collection, "Skill collection is empty") + if self._skill_collection is None: + raise ValueError("Skill collection cannot be empty") assert self._skill_collection is not None context = SKContext( diff --git a/python/semantic_kernel/orchestration/sk_function_base.py b/python/semantic_kernel/orchestration/sk_function_base.py index a49d20dc17c5..7bb06ea32292 100644 --- a/python/semantic_kernel/orchestration/sk_function_base.py +++ b/python/semantic_kernel/orchestration/sk_function_base.py @@ -18,6 +18,10 @@ class SKFunctionBase(ABC): + FUNCTION_PARAM_NAME_REGEX = r"^[0-9A-Za-z_]*$" + FUNCTION_NAME_REGEX = r"^[0-9A-Za-z_]*$" + SKILL_NAME_REGEX = r"^[0-9A-Za-z_]*$" + @property @abstractmethod def name(self) -> str: diff --git a/python/semantic_kernel/skill_definition/function_view.py b/python/semantic_kernel/skill_definition/function_view.py index 0d64cec15c93..abf37c6a6dbc 100644 --- a/python/semantic_kernel/skill_definition/function_view.py +++ b/python/semantic_kernel/skill_definition/function_view.py @@ -1,8 +1,9 @@ # Copyright (c) Microsoft. All rights reserved. from typing import List +from re import match as re_match -from semantic_kernel.diagnostics.verify import Verify +from semantic_kernel.orchestration.sk_function_base import SKFunctionBase from semantic_kernel.skill_definition.parameter_view import ParameterView @@ -23,7 +24,11 @@ def __init__( is_semantic: bool, is_asynchronous: bool = True, ) -> None: - Verify.valid_function_name(name) + if not re_match(SKFunctionBase.FUNCTION_NAME_REGEX, name): + raise ValueError( + f"Invalid function name: {name}. Function names " + f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" + ) self._name = name self._skill_name = skill_name @@ -58,7 +63,11 @@ def is_asynchronous(self) -> bool: @name.setter def name(self, value: str) -> None: - Verify.valid_function_name(value) + if not re_match(SKFunctionBase.FUNCTION_NAME_REGEX, value): + raise ValueError( + f"Invalid function name: {value}. Function names " + f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" + ) self._name = value @skill_name.setter diff --git a/python/semantic_kernel/skill_definition/parameter_view.py b/python/semantic_kernel/skill_definition/parameter_view.py index c39c145148f2..1cbb2254b804 100644 --- a/python/semantic_kernel/skill_definition/parameter_view.py +++ b/python/semantic_kernel/skill_definition/parameter_view.py @@ -1,6 +1,8 @@ # Copyright (c) Microsoft. All rights reserved. -from semantic_kernel.diagnostics.verify import Verify +from re import match as re_match + +from semantic_kernel.orchestration.sk_function_base import SKFunctionBase class ParameterView: @@ -9,7 +11,11 @@ class ParameterView: _default_value: str def __init__(self, name: str, description: str, default_value: str) -> None: - Verify.valid_function_param_name(name) + if not re_match(SKFunctionBase.FUNCTION_PARAM_NAME_REGEX, name): + raise ValueError( + f"Invalid function parameter name: {name}. Function parameter names " + f"must match the regex: {SKFunctionBase.FUNCTION_PARAM_NAME_REGEX}" + ) self._name = name self._description = description @@ -29,7 +35,11 @@ def default_value(self) -> str: @name.setter def name(self, value: str) -> None: - Verify.valid_function_param_name(value) + if not re_match(SKFunctionBase.FUNCTION_PARAM_NAME_REGEX, value): + raise ValueError( + f"Invalid function parameter name: {value}. Function parameter names " + f"must match the regex: {SKFunctionBase.FUNCTION_PARAM_NAME_REGEX}" + ) self._name = value @description.setter diff --git a/python/semantic_kernel/skill_definition/skill_collection.py b/python/semantic_kernel/skill_definition/skill_collection.py index 448d7c0e0f10..51f178253cea 100644 --- a/python/semantic_kernel/skill_definition/skill_collection.py +++ b/python/semantic_kernel/skill_definition/skill_collection.py @@ -3,7 +3,6 @@ from logging import Logger from typing import TYPE_CHECKING, Dict, Literal, Optional, Tuple -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_exception import KernelException from semantic_kernel.skill_definition.functions_view import FunctionsView from semantic_kernel.skill_definition.read_only_skill_collection import ( @@ -35,7 +34,8 @@ def __init__(self, log: Optional[Logger] = None) -> None: self._skill_collection = {} def add_semantic_function(self, function: "SKFunctionBase") -> None: - Verify.not_null(function, "The function provided is None") + if function is None: + raise ValueError("The function provided cannot be None") s_name, f_name = function.skill_name, function.name s_name, f_name = s_name.lower(), f_name.lower() @@ -46,7 +46,8 @@ def add_semantic_function(self, function: "SKFunctionBase") -> None: self._skill_collection[s_name][f_name] = function def add_native_function(self, function: "SKFunctionBase") -> None: - Verify.not_null(function, "The function provided is None") + if function is None: + raise ValueError("The function provided cannot be None") s_name, f_name = function.skill_name, function.name s_name, f_name = self._normalize_names(s_name, f_name, True) @@ -141,8 +142,8 @@ def _normalize_names( if s_name is None and allow_substitution: s_name = self.GLOBAL_SKILL - Verify.not_null(s_name, "The skill name provided is None") - assert s_name is not None # to make type checker happy + if s_name is None: + raise ValueError("The skill name provided cannot be None") s_name, f_name = s_name.lower(), f_name.lower() return s_name, f_name From e6694bfbacdd66f767c8dedd1dd399cc52aa95fb Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Mon, 3 Apr 2023 12:20:35 -0500 Subject: [PATCH 03/10] No more diagonstics sub-module --- .../diagnostics/sk_exception.py | 29 -------- .../semantic_kernel/diagnostics/validation.py | 70 ------------------- .../diagnostics/validation_exception.py | 50 ------------- 3 files changed, 149 deletions(-) delete mode 100644 python/semantic_kernel/diagnostics/sk_exception.py delete mode 100644 python/semantic_kernel/diagnostics/validation.py delete mode 100644 python/semantic_kernel/diagnostics/validation_exception.py diff --git a/python/semantic_kernel/diagnostics/sk_exception.py b/python/semantic_kernel/diagnostics/sk_exception.py deleted file mode 100644 index b5743bb3229f..000000000000 --- a/python/semantic_kernel/diagnostics/sk_exception.py +++ /dev/null @@ -1,29 +0,0 @@ -# Copyright (c) Microsoft. All rights reserved. - -from enum import Enum -from typing import Optional - - -class SKException(Exception): - """The base class for all semantic kernel exceptions.""" - - def __init__( - self, - error_code: Enum, - message: Optional[str] = None, - inner_exception: Optional[Exception] = None, - ) -> None: - """Initializes a new instance of the SKException class. - - Arguments: - error_code {Enum} -- The error code. - message {str} -- The error message. - inner_exception {Exception} -- The inner exception. - """ - super().__init__(self._build_message(error_code, message), inner_exception) - - def _build_message(self, error_code: Enum, message: Optional[str]) -> str: - if message is None: - return error_code.name - else: - return f"{error_code.name}: {message}" diff --git a/python/semantic_kernel/diagnostics/validation.py b/python/semantic_kernel/diagnostics/validation.py deleted file mode 100644 index 5dea044ad491..000000000000 --- a/python/semantic_kernel/diagnostics/validation.py +++ /dev/null @@ -1,70 +0,0 @@ -# Copyright (c) Microsoft. All rights reserved. - -from re import match as re_match -from typing import Optional - - -def valid_skill_name(value: Optional[str]) -> None: - """ - Validates that the skill name is valid. - - Valid skill names are non-empty and - match the regex: [0-9A-Za-z_]* - - :param value: The skill name to validate. - - :raises ValueError: If the skill name is invalid. - """ - if not value: - raise ValueError("The skill name cannot be empty") - - SKILL_NAME_REGEX = r"^[0-9A-Za-z_]*$" - if not re_match(SKILL_NAME_REGEX, value): - raise ValueError( - f"Invalid skill name: {value}. Skill " - f"names must match the regex: {SKILL_NAME_REGEX}" - ) - - -def valid_function_name(value: Optional[str]) -> None: - """ - Validates that the function name is valid. - - Valid function names are non-empty and - match the regex: [0-9A-Za-z_]* - - :param value: The function name to validate. - - :raises ValueError: If the function name is invalid. - """ - if not value: - raise ValueError("The function name cannot be empty") - - FUNCTION_NAME_REGEX = r"^[0-9A-Za-z_]*$" - if not re_match(FUNCTION_NAME_REGEX, value): - raise ValueError( - f"Invalid function name: {value}. Function " - f"names must match the regex: {FUNCTION_NAME_REGEX}" - ) - - -def valid_function_param_name(value: Optional[str]) -> None: - """ - Validates that the function parameter name is valid. - - Valid function parameter names are non-empty and - match the regex: [0-9A-Za-z_]* - - :param value: The function parameter name to validate. - - :raises ValueError: If the function parameter name is invalid. - """ - if not value: - raise ValueError("The function parameter name cannot be empty") - - FUNCTION_PARAM_NAME_REGEX = r"^[0-9A-Za-z_]*$" - if not re_match(FUNCTION_PARAM_NAME_REGEX, value): - raise ValueError( - f"Invalid function parameter name: {value}. Function parameter " - f"names must match the regex: {FUNCTION_PARAM_NAME_REGEX}" - ) diff --git a/python/semantic_kernel/diagnostics/validation_exception.py b/python/semantic_kernel/diagnostics/validation_exception.py deleted file mode 100644 index 2a7a86889a5c..000000000000 --- a/python/semantic_kernel/diagnostics/validation_exception.py +++ /dev/null @@ -1,50 +0,0 @@ -# Copyright (c) Microsoft. All rights reserved. - -from enum import Enum -from typing import Optional - -from semantic_kernel.diagnostics.sk_exception import SKException - - -class ValidationException(SKException): - class ErrorCodes(Enum): - # Unknown error. - UnknownError = -1 - # Null value. - NullValue = 0 - # Empty value. - EmptyValue = 1 - # Out of range. - OutOfRange = 2 - # Missing prefix. - MissingPrefix = 3 - # Directory not found. - DirectoryNotFound = 4 - - # The error code. - _error_code: ErrorCodes - - def __init__( - self, - error_code: ErrorCodes, - message: str, - inner_exception: Optional[Exception] = None, - ) -> None: - """Initializes a new instance of the ValidationException class. - - Arguments: - error_code {ErrorCodes} -- The error code. - message {str} -- The error message. - inner_exception {Exception} -- The inner exception. - """ - super().__init__(error_code, message, inner_exception) - self._error_code = error_code - - @property - def error_code(self) -> ErrorCodes: - """Gets the error code. - - Returns: - ErrorCodes -- The error code. - """ - return self._error_code From 197ee81cc1e48e0cb031cbe0d0553d9acaeb1681 Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Mon, 3 Apr 2023 12:20:46 -0500 Subject: [PATCH 04/10] Continue re-write sans Verify --- python/semantic_kernel/ai/ai_exception.py | 4 +- python/semantic_kernel/kernel_exception.py | 4 +- .../import_semantic_skill_from_directory.py | 8 +-- .../inline_function_definitions.py | 17 ++--- .../skill_definition/function_view.py | 6 +- .../skill_definition/parameter_view.py | 15 +--- .../blocks/function_id_block.py | 13 ++-- python/semantic_kernel/utils/validation.py | 70 +++++++++++++++++++ 8 files changed, 91 insertions(+), 46 deletions(-) create mode 100644 python/semantic_kernel/utils/validation.py diff --git a/python/semantic_kernel/ai/ai_exception.py b/python/semantic_kernel/ai/ai_exception.py index 0e1dcd9f1ce4..255e4a87ccfc 100644 --- a/python/semantic_kernel/ai/ai_exception.py +++ b/python/semantic_kernel/ai/ai_exception.py @@ -3,10 +3,8 @@ from enum import Enum from typing import Optional -from semantic_kernel.diagnostics.sk_exception import SKException - -class AIException(SKException): +class AIException(Exception): class ErrorCodes(Enum): # Unknown error. UnknownError = -1 diff --git a/python/semantic_kernel/kernel_exception.py b/python/semantic_kernel/kernel_exception.py index 7c73a1a74552..cb39508f9a3f 100644 --- a/python/semantic_kernel/kernel_exception.py +++ b/python/semantic_kernel/kernel_exception.py @@ -3,10 +3,8 @@ from enum import Enum from typing import Optional -from semantic_kernel.diagnostics.sk_exception import SKException - -class KernelException(SKException): +class KernelException(Exception): class ErrorCodes(Enum): # Unknown error. UnknownError = -1 diff --git a/python/semantic_kernel/kernel_extensions/import_semantic_skill_from_directory.py b/python/semantic_kernel/kernel_extensions/import_semantic_skill_from_directory.py index 73c331b94cb7..33763b07caef 100644 --- a/python/semantic_kernel/kernel_extensions/import_semantic_skill_from_directory.py +++ b/python/semantic_kernel/kernel_extensions/import_semantic_skill_from_directory.py @@ -3,7 +3,6 @@ import glob import os from typing import Dict -from re import match as re_match from semantic_kernel.kernel_base import KernelBase from semantic_kernel.orchestration.sk_function_base import SKFunctionBase @@ -14,6 +13,7 @@ from semantic_kernel.semantic_functions.semantic_function_config import ( SemanticFunctionConfig, ) +from semantic_kernel.utils.validation import validate_skill_name def import_semantic_skill_from_directory( @@ -22,11 +22,7 @@ def import_semantic_skill_from_directory( CONFIG_FILE = "config.json" PROMPT_FILE = "skprompt.txt" - if not re_match(SKFunctionBase.SKILL_NAME_REGEX, skill_directory_name): - raise ValueError( - f"Invalid skill name: {skill_directory_name}. Skill names " - f"must match the regex: {SKFunctionBase.SKILL_NAME_REGEX}" - ) + validate_skill_name(skill_directory_name) skill_directory = os.path.join(parent_directory, skill_directory_name) skill_directory = os.path.abspath(skill_directory) diff --git a/python/semantic_kernel/kernel_extensions/inline_function_definitions.py b/python/semantic_kernel/kernel_extensions/inline_function_definitions.py index 074ef2507d25..741265842fdb 100644 --- a/python/semantic_kernel/kernel_extensions/inline_function_definitions.py +++ b/python/semantic_kernel/kernel_extensions/inline_function_definitions.py @@ -2,7 +2,6 @@ from typing import TYPE_CHECKING, List, Optional from uuid import uuid4 -from re import match as re_match from semantic_kernel.kernel_base import KernelBase from semantic_kernel.semantic_functions.prompt_template import PromptTemplate @@ -12,6 +11,10 @@ from semantic_kernel.semantic_functions.semantic_function_config import ( SemanticFunctionConfig, ) +from semantic_kernel.utils.validation import ( + validate_function_name, + validate_skill_name, +) if TYPE_CHECKING: from semantic_kernel.orchestration.sk_function_base import SKFunctionBase @@ -53,17 +56,9 @@ def create_semantic_function( ), ) - if not re_match(SKFunctionBase.FUNCTION_NAME_REGEX, function_name): - raise ValueError( - f"Invalid function name: {function_name}. Function names " - f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" - ) + validate_function_name(function_name) if skill_name is not None: - if not re_match(SKFunctionBase.SKILL_NAME_REGEX, skill_name): - raise ValueError( - f"Invalid skill name: {skill_name}. Skill names " - f"must match the regex: {SKFunctionBase.SKILL_NAME_REGEX}" - ) + validate_skill_name(skill_name) template = PromptTemplate(prompt_template, kernel.prompt_template_engine, config) function_config = SemanticFunctionConfig(config, template) diff --git a/python/semantic_kernel/skill_definition/function_view.py b/python/semantic_kernel/skill_definition/function_view.py index abf37c6a6dbc..efa3ca30a57a 100644 --- a/python/semantic_kernel/skill_definition/function_view.py +++ b/python/semantic_kernel/skill_definition/function_view.py @@ -1,10 +1,10 @@ # Copyright (c) Microsoft. All rights reserved. from typing import List -from re import match as re_match from semantic_kernel.orchestration.sk_function_base import SKFunctionBase from semantic_kernel.skill_definition.parameter_view import ParameterView +from semantic_kernel.utils.validation import valid_function_name class FunctionView: @@ -24,7 +24,7 @@ def __init__( is_semantic: bool, is_asynchronous: bool = True, ) -> None: - if not re_match(SKFunctionBase.FUNCTION_NAME_REGEX, name): + if not valid_function_name(name): raise ValueError( f"Invalid function name: {name}. Function names " f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" @@ -63,7 +63,7 @@ def is_asynchronous(self) -> bool: @name.setter def name(self, value: str) -> None: - if not re_match(SKFunctionBase.FUNCTION_NAME_REGEX, value): + if not valid_function_name(value): raise ValueError( f"Invalid function name: {value}. Function names " f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" diff --git a/python/semantic_kernel/skill_definition/parameter_view.py b/python/semantic_kernel/skill_definition/parameter_view.py index 1cbb2254b804..cbb0fd0c4fdd 100644 --- a/python/semantic_kernel/skill_definition/parameter_view.py +++ b/python/semantic_kernel/skill_definition/parameter_view.py @@ -1,8 +1,7 @@ # Copyright (c) Microsoft. All rights reserved. -from re import match as re_match - from semantic_kernel.orchestration.sk_function_base import SKFunctionBase +from semantic_kernel.utils.validation import validate_function_param_name class ParameterView: @@ -11,11 +10,7 @@ class ParameterView: _default_value: str def __init__(self, name: str, description: str, default_value: str) -> None: - if not re_match(SKFunctionBase.FUNCTION_PARAM_NAME_REGEX, name): - raise ValueError( - f"Invalid function parameter name: {name}. Function parameter names " - f"must match the regex: {SKFunctionBase.FUNCTION_PARAM_NAME_REGEX}" - ) + validate_function_param_name(name) self._name = name self._description = description @@ -35,11 +30,7 @@ def default_value(self) -> str: @name.setter def name(self, value: str) -> None: - if not re_match(SKFunctionBase.FUNCTION_PARAM_NAME_REGEX, value): - raise ValueError( - f"Invalid function parameter name: {value}. Function parameter names " - f"must match the regex: {SKFunctionBase.FUNCTION_PARAM_NAME_REGEX}" - ) + validate_function_param_name(value) self._name = value @description.setter diff --git a/python/semantic_kernel/template_engine/blocks/function_id_block.py b/python/semantic_kernel/template_engine/blocks/function_id_block.py index 655eea90ba1f..340c5daca2d9 100644 --- a/python/semantic_kernel/template_engine/blocks/function_id_block.py +++ b/python/semantic_kernel/template_engine/blocks/function_id_block.py @@ -1,13 +1,13 @@ # Copyright (c) Microsoft. All rights reserved. from logging import Logger -from re import match as re_match from typing import Optional, Tuple from semantic_kernel.orchestration.context_variables import ContextVariables from semantic_kernel.template_engine.blocks.block import Block from semantic_kernel.template_engine.blocks.block_types import BlockTypes from semantic_kernel.template_engine.protocols.text_renderer import TextRenderer +from semantic_kernel.utils.validation import validate_function_name class FunctionIdBlock(Block, TextRenderer): @@ -38,13 +38,10 @@ def is_valid(self) -> Tuple[bool, str]: error_msg = "The function identifier is empty" return False, error_msg - if not re_match(r"^[a-zA-Z0-9_.]*$", self.content): - error_msg = ( - f"The function identifier '{self.content}' contains invalid " - "characters. Only alphanumeric chars, underscore and a single " - "dot are allowed." - ) - return False, error_msg + try: + validate_function_name(self.content) + except ValueError as ex: + return False, str(ex) if self._has_more_than_one_dot(self.content): error_msg = ( diff --git a/python/semantic_kernel/utils/validation.py b/python/semantic_kernel/utils/validation.py new file mode 100644 index 000000000000..3bc8914baa7f --- /dev/null +++ b/python/semantic_kernel/utils/validation.py @@ -0,0 +1,70 @@ +# Copyright (c) Microsoft. All rights reserved. + +from re import match as re_match +from typing import Optional + + +def validate_skill_name(value: Optional[str]) -> None: + """ + Validates that the skill name is valid. + + Valid skill names are non-empty and + match the regex: [0-9A-Za-z_]* + + :param value: The skill name to validate. + + :raises ValueError: If the skill name is invalid. + """ + if not value: + raise ValueError("The skill name cannot be empty") + + SKILL_NAME_REGEX = r"^[0-9A-Za-z_]*$" + if not re_match(SKILL_NAME_REGEX, value): + raise ValueError( + f"Invalid skill name: {value}. Skill " + f"names must match the regex: {SKILL_NAME_REGEX}" + ) + + +def validate_function_name(value: Optional[str]) -> None: + """ + Validates that the function name is valid. + + Valid function names are non-empty and + match the regex: [0-9A-Za-z_]* + + :param value: The function name to validate. + + :raises ValueError: If the function name is invalid. + """ + if not value: + raise ValueError("The function name cannot be empty") + + FUNCTION_NAME_REGEX = r"^[0-9A-Za-z_]*$" + if not re_match(FUNCTION_NAME_REGEX, value): + raise ValueError( + f"Invalid function name: {value}. Function " + f"names must match the regex: {FUNCTION_NAME_REGEX}" + ) + + +def validate_function_param_name(value: Optional[str]) -> None: + """ + Validates that the function parameter name is valid. + + Valid function parameter names are non-empty and + match the regex: [0-9A-Za-z_]* + + :param value: The function parameter name to validate. + + :raises ValueError: If the function parameter name is invalid. + """ + if not value: + raise ValueError("The function parameter name cannot be empty") + + FUNCTION_PARAM_NAME_REGEX = r"^[0-9A-Za-z_]*$" + if not re_match(FUNCTION_PARAM_NAME_REGEX, value): + raise ValueError( + f"Invalid function parameter name: {value}. Function parameter " + f"names must match the regex: {FUNCTION_PARAM_NAME_REGEX}" + ) From d7dfe83ce32d695157716e71516f52c58a9b832b Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Mon, 3 Apr 2023 12:26:17 -0500 Subject: [PATCH 05/10] Continue transition, make sure tests pass, lint --- python/semantic_kernel/core_skills/__init__.py | 4 ++-- .../semantic_kernel/core_skills/file_io_skill.py | 7 ++----- python/semantic_kernel/kernel.py | 12 ++---------- .../inline_function_definitions.py | 5 +---- .../skill_definition/function_view.py | 16 ++++------------ .../skill_definition/parameter_view.py | 1 - .../template_engine/blocks/function_id_block.py | 15 ++++++++++----- python/tests/test_time_skill.py | 3 +-- 8 files changed, 22 insertions(+), 41 deletions(-) diff --git a/python/semantic_kernel/core_skills/__init__.py b/python/semantic_kernel/core_skills/__init__.py index bd2b52b790eb..a6ae27ae42dd 100644 --- a/python/semantic_kernel/core_skills/__init__.py +++ b/python/semantic_kernel/core_skills/__init__.py @@ -1,8 +1,8 @@ # Copyright (c) Microsoft. All rights reserved. -from semantic_kernel.core_skills.text_memory_skill import TextMemorySkill from semantic_kernel.core_skills.file_io_skill import FileIOSkill -from semantic_kernel.core_skills.time_skill import TimeSkill +from semantic_kernel.core_skills.text_memory_skill import TextMemorySkill from semantic_kernel.core_skills.text_skill import TextSkill +from semantic_kernel.core_skills.time_skill import TimeSkill __all__ = ["TextMemorySkill", "TextSkill", "FileIOSkill", "TimeSkill"] diff --git a/python/semantic_kernel/core_skills/file_io_skill.py b/python/semantic_kernel/core_skills/file_io_skill.py index 20c0ff7a04d5..d90754cdb0df 100644 --- a/python/semantic_kernel/core_skills/file_io_skill.py +++ b/python/semantic_kernel/core_skills/file_io_skill.py @@ -1,12 +1,9 @@ import os import aiofiles -from semantic_kernel.orchestration.sk_context import SKContext -from semantic_kernel.skill_definition import ( - sk_function, - sk_function_context_parameter, -) +from semantic_kernel.orchestration.sk_context import SKContext +from semantic_kernel.skill_definition import sk_function, sk_function_context_parameter class FileIOSkill: diff --git a/python/semantic_kernel/kernel.py b/python/semantic_kernel/kernel.py index 43a53f5708ce..39eb259dc827 100644 --- a/python/semantic_kernel/kernel.py +++ b/python/semantic_kernel/kernel.py @@ -36,6 +36,7 @@ from semantic_kernel.template_engine.protocols.prompt_templating_engine import ( PromptTemplatingEngine, ) +from semantic_kernel.utils.validation import validate_function_name, validate_skill_name class Kernel(KernelBase): @@ -90,16 +91,7 @@ def register_semantic_function( assert skill_name is not None # for type checker validate_skill_name(skill_name) - if not re_match(SKFunctionBase.SKILL_NAME_REGEX, skill_name): - raise ValueError( - f"Invalid skill name: {skill_name}. Skill names " - f"must match the regex: {SKFunctionBase.SKILL_NAME_REGEX}" - ) - if not re_match(SKFunctionBase.FUNCTION_NAME_REGEX, function_name): - raise ValueError( - f"Invalid function name: {function_name}. Function names " - f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" - ) + validate_function_name(function_name) function = self._create_semantic_function( skill_name, function_name, function_config diff --git a/python/semantic_kernel/kernel_extensions/inline_function_definitions.py b/python/semantic_kernel/kernel_extensions/inline_function_definitions.py index 741265842fdb..f569e012b48e 100644 --- a/python/semantic_kernel/kernel_extensions/inline_function_definitions.py +++ b/python/semantic_kernel/kernel_extensions/inline_function_definitions.py @@ -11,10 +11,7 @@ from semantic_kernel.semantic_functions.semantic_function_config import ( SemanticFunctionConfig, ) -from semantic_kernel.utils.validation import ( - validate_function_name, - validate_skill_name, -) +from semantic_kernel.utils.validation import validate_function_name, validate_skill_name if TYPE_CHECKING: from semantic_kernel.orchestration.sk_function_base import SKFunctionBase diff --git a/python/semantic_kernel/skill_definition/function_view.py b/python/semantic_kernel/skill_definition/function_view.py index efa3ca30a57a..12539178487e 100644 --- a/python/semantic_kernel/skill_definition/function_view.py +++ b/python/semantic_kernel/skill_definition/function_view.py @@ -2,9 +2,8 @@ from typing import List -from semantic_kernel.orchestration.sk_function_base import SKFunctionBase from semantic_kernel.skill_definition.parameter_view import ParameterView -from semantic_kernel.utils.validation import valid_function_name +from semantic_kernel.utils.validation import validate_function_name class FunctionView: @@ -24,11 +23,7 @@ def __init__( is_semantic: bool, is_asynchronous: bool = True, ) -> None: - if not valid_function_name(name): - raise ValueError( - f"Invalid function name: {name}. Function names " - f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" - ) + validate_function_name(name) self._name = name self._skill_name = skill_name @@ -63,11 +58,8 @@ def is_asynchronous(self) -> bool: @name.setter def name(self, value: str) -> None: - if not valid_function_name(value): - raise ValueError( - f"Invalid function name: {value}. Function names " - f"must match the regex: {SKFunctionBase.FUNCTION_NAME_REGEX}" - ) + validate_function_name(value) + self._name = value @skill_name.setter diff --git a/python/semantic_kernel/skill_definition/parameter_view.py b/python/semantic_kernel/skill_definition/parameter_view.py index cbb0fd0c4fdd..41ad4a19401a 100644 --- a/python/semantic_kernel/skill_definition/parameter_view.py +++ b/python/semantic_kernel/skill_definition/parameter_view.py @@ -1,6 +1,5 @@ # Copyright (c) Microsoft. All rights reserved. -from semantic_kernel.orchestration.sk_function_base import SKFunctionBase from semantic_kernel.utils.validation import validate_function_param_name diff --git a/python/semantic_kernel/template_engine/blocks/function_id_block.py b/python/semantic_kernel/template_engine/blocks/function_id_block.py index 340c5daca2d9..241c558d0b83 100644 --- a/python/semantic_kernel/template_engine/blocks/function_id_block.py +++ b/python/semantic_kernel/template_engine/blocks/function_id_block.py @@ -1,13 +1,13 @@ # Copyright (c) Microsoft. All rights reserved. from logging import Logger +from re import match as re_match from typing import Optional, Tuple from semantic_kernel.orchestration.context_variables import ContextVariables from semantic_kernel.template_engine.blocks.block import Block from semantic_kernel.template_engine.blocks.block_types import BlockTypes from semantic_kernel.template_engine.protocols.text_renderer import TextRenderer -from semantic_kernel.utils.validation import validate_function_name class FunctionIdBlock(Block, TextRenderer): @@ -38,10 +38,15 @@ def is_valid(self) -> Tuple[bool, str]: error_msg = "The function identifier is empty" return False, error_msg - try: - validate_function_name(self.content) - except ValueError as ex: - return False, str(ex) + if not re_match(r"^[a-zA-Z0-9_.]*$", self.content): + # NOTE: this is not quite the same as + # utils.validation.validate_function_name + error_msg = ( + f"The function identifier '{self.content}' contains invalid " + "characters. Only alphanumeric chars, underscore and a single " + "dot are allowed." + ) + return False, error_msg if self._has_more_than_one_dot(self.content): error_msg = ( diff --git a/python/tests/test_time_skill.py b/python/tests/test_time_skill.py index 1e2c51d584aa..6a86baa148b3 100644 --- a/python/tests/test_time_skill.py +++ b/python/tests/test_time_skill.py @@ -1,8 +1,7 @@ import datetime -import semantic_kernel as sk - from unittest import mock +import semantic_kernel as sk from semantic_kernel.core_skills.time_skill import TimeSkill test_mock_now = datetime.datetime(2031, 1, 12, 12, 24, 56, tzinfo=datetime.timezone.utc) From 33bdbbe9e4ca7a312ace4e9f1980c84f21bdb643 Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Mon, 3 Apr 2023 12:36:59 -0500 Subject: [PATCH 06/10] Consistency pass for ValueErrors --- .../ai/open_ai/services/azure_chat_completion.py | 6 +++--- .../ai/open_ai/services/azure_open_ai_config.py | 10 +++++----- .../ai/open_ai/services/azure_text_completion.py | 6 +++--- .../ai/open_ai/services/azure_text_embedding.py | 6 +++--- .../open_ai/services/open_ai_chat_completion.py | 2 +- .../ai/open_ai/services/open_ai_config.py | 4 ++-- .../open_ai/services/open_ai_text_completion.py | 4 ++-- .../configuration/kernel_config.py | 12 ++++++------ python/semantic_kernel/kernel.py | 15 ++++++++++++--- python/semantic_kernel/kernel_builder.py | 8 ++++---- .../kernel_extensions/memory_configuration.py | 6 +++--- .../orchestration/context_variables.py | 4 ++-- .../semantic_kernel/orchestration/sk_function.py | 16 ++++++++-------- .../skill_definition/skill_collection.py | 6 +++--- python/semantic_kernel/utils/validation.py | 6 +++--- 15 files changed, 60 insertions(+), 51 deletions(-) diff --git a/python/semantic_kernel/ai/open_ai/services/azure_chat_completion.py b/python/semantic_kernel/ai/open_ai/services/azure_chat_completion.py index eb8d395da24a..d4a71632361b 100644 --- a/python/semantic_kernel/ai/open_ai/services/azure_chat_completion.py +++ b/python/semantic_kernel/ai/open_ai/services/azure_chat_completion.py @@ -22,11 +22,11 @@ def __init__( logger: Optional[Logger] = None, ) -> None: if not deployment_name: - raise ValueError("The deployment name cannot be empty") + raise ValueError("The deployment name cannot be `None` or empty") if not api_key: - raise ValueError("The Azure API key cannot be empty") + raise ValueError("The Azure API key cannot be `None` or empty`") if not endpoint: - raise ValueError("The Azure endpoint cannot be empty") + raise ValueError("The Azure endpoint cannot be `None` or empty") if not endpoint.startswith("https://"): raise ValueError("The Azure endpoint must start with https://") diff --git a/python/semantic_kernel/ai/open_ai/services/azure_open_ai_config.py b/python/semantic_kernel/ai/open_ai/services/azure_open_ai_config.py index bd560a432c22..1cd5c5c26ac8 100644 --- a/python/semantic_kernel/ai/open_ai/services/azure_open_ai_config.py +++ b/python/semantic_kernel/ai/open_ai/services/azure_open_ai_config.py @@ -40,13 +40,13 @@ def __init__( https://learn.microsoft.com/azure/cognitive-services/openai/reference """ if not deployment_name: - raise ValueError("The deployment name cannot be empty") + raise ValueError("The deployment name cannot be `None` or empty") + if not api_key: + raise ValueError("The Azure API key cannot be `None` or empty`") if not endpoint: - raise ValueError("The endpoint cannot be empty") + raise ValueError("The Azure endpoint cannot be `None` or empty") if not endpoint.startswith("https://"): - raise ValueError("The endpoint must start with https://") - if not api_key: - raise ValueError("The API key cannot be empty") + raise ValueError("The Azure endpoint must start with https://") self.deployment_name = deployment_name self.endpoint = endpoint diff --git a/python/semantic_kernel/ai/open_ai/services/azure_text_completion.py b/python/semantic_kernel/ai/open_ai/services/azure_text_completion.py index f0431e7a5d9a..420e6c49ddda 100644 --- a/python/semantic_kernel/ai/open_ai/services/azure_text_completion.py +++ b/python/semantic_kernel/ai/open_ai/services/azure_text_completion.py @@ -22,11 +22,11 @@ def __init__( logger: Optional[Logger] = None, ) -> None: if not deployment_name: - raise ValueError("The deployment name cannot be empty") + raise ValueError("The deployment name cannot be `None` or empty") if not api_key: - raise ValueError("The Azure API key cannot be empty") + raise ValueError("The Azure API key cannot be `None` or empty`") if not endpoint: - raise ValueError("The Azure endpoint cannot be empty") + raise ValueError("The Azure endpoint cannot be `None` or empty") if not endpoint.startswith("https://"): raise ValueError("The Azure endpoint must start with https://") diff --git a/python/semantic_kernel/ai/open_ai/services/azure_text_embedding.py b/python/semantic_kernel/ai/open_ai/services/azure_text_embedding.py index a5728a48a1ad..dc6778663c35 100644 --- a/python/semantic_kernel/ai/open_ai/services/azure_text_embedding.py +++ b/python/semantic_kernel/ai/open_ai/services/azure_text_embedding.py @@ -22,11 +22,11 @@ def __init__( logger: Optional[Logger] = None, ) -> None: if not deployment_name: - raise ValueError("The deployment name cannot be empty") + raise ValueError("The deployment name cannot be `None` or empty") if not api_key: - raise ValueError("The Azure API key cannot be empty") + raise ValueError("The Azure API key cannot be `None` or empty`") if not endpoint: - raise ValueError("The Azure endpoint cannot be empty") + raise ValueError("The Azure endpoint cannot be `None` or empty") if not endpoint.startswith("https://"): raise ValueError("The Azure endpoint must start with https://") diff --git a/python/semantic_kernel/ai/open_ai/services/open_ai_chat_completion.py b/python/semantic_kernel/ai/open_ai/services/open_ai_chat_completion.py index 61bf39b30c00..7043d03d345b 100644 --- a/python/semantic_kernel/ai/open_ai/services/open_ai_chat_completion.py +++ b/python/semantic_kernel/ai/open_ai/services/open_ai_chat_completion.py @@ -65,7 +65,7 @@ async def complete_chat_async( str -- The completed text. """ if request_settings is None: - raise ValueError("The request settings cannot be empty") + raise ValueError("The request settings cannot be `None`") if request_settings.max_tokens < 1: raise AIException( diff --git a/python/semantic_kernel/ai/open_ai/services/open_ai_config.py b/python/semantic_kernel/ai/open_ai/services/open_ai_config.py index dc7a4dfb1e47..dfb42ae85275 100644 --- a/python/semantic_kernel/ai/open_ai/services/open_ai_config.py +++ b/python/semantic_kernel/ai/open_ai/services/open_ai_config.py @@ -32,9 +32,9 @@ def __init__( account belongs to multiple organizations. """ if not model_id: - raise ValueError("The model ID cannot be empty") + raise ValueError("The model ID cannot be `None` or empty") if not api_key: - raise ValueError("The API key cannot be empty") + raise ValueError("The API key cannot be `None` or empty") self.model_id = model_id self.api_key = api_key diff --git a/python/semantic_kernel/ai/open_ai/services/open_ai_text_completion.py b/python/semantic_kernel/ai/open_ai/services/open_ai_text_completion.py index b3b4db676e5d..61b424863efb 100644 --- a/python/semantic_kernel/ai/open_ai/services/open_ai_text_completion.py +++ b/python/semantic_kernel/ai/open_ai/services/open_ai_text_completion.py @@ -65,9 +65,9 @@ async def complete_simple_async( str -- The completed text. """ if not prompt: - raise ValueError("The prompt cannot be empty") + raise ValueError("The prompt cannot be `None` or empty") if request_settings is None: - raise ValueError("The request settings cannot be empty") + raise ValueError("The request settings cannot be `None`") if request_settings.max_tokens < 1: raise AIException( diff --git a/python/semantic_kernel/configuration/kernel_config.py b/python/semantic_kernel/configuration/kernel_config.py index ce262aca18fa..01fa53bc3e2a 100644 --- a/python/semantic_kernel/configuration/kernel_config.py +++ b/python/semantic_kernel/configuration/kernel_config.py @@ -31,7 +31,7 @@ def add_openai_chat_backend( overwrite: bool = False, ) -> "KernelConfig": if not name: - raise ValueError("The backend name cannot be empty") + raise ValueError("The backend name cannot be `None` or empty") if not overwrite and name in self._chat_backends: raise KernelException( @@ -59,7 +59,7 @@ def add_azure_openai_chat_backend( overwrite: bool = False, ) -> "KernelConfig": if not name: - raise ValueError("The backend name cannot be empty") + raise ValueError("The backend name cannot be `None` or empty") if not overwrite and name in self._chat_backends: raise KernelException( @@ -89,7 +89,7 @@ def add_azure_openai_completion_backend( overwrite: bool = False, ) -> "KernelConfig": if not name: - raise ValueError("The backend name cannot be empty") + raise ValueError("The backend name cannot be `None` or empty") if not overwrite and name in self._completion_backends: raise KernelException( @@ -118,7 +118,7 @@ def add_openai_completion_backend( overwrite: bool = False, ) -> "KernelConfig": if not name: - raise ValueError("The backend name cannot be empty") + raise ValueError("The backend name cannot be `None` or empty") if not overwrite and name in self._completion_backends: raise KernelException( @@ -146,7 +146,7 @@ def add_azure_open_ai_embeddings_backend( overwrite: bool = False, ) -> "KernelConfig": if not name: - raise ValueError("The backend name cannot be empty") + raise ValueError("The backend name cannot be `None` or empty") if not overwrite and name in self._embeddings_backends: raise KernelException( @@ -175,7 +175,7 @@ def add_open_ai_embeddings_backend( overwrite: bool = False, ) -> "KernelConfig": if not name: - raise ValueError("The backend name cannot be empty") + raise ValueError("The backend name cannot be `None` or empty") if not overwrite and name in self._embeddings_backends: raise KernelException( diff --git a/python/semantic_kernel/kernel.py b/python/semantic_kernel/kernel.py index 39eb259dc827..5bdbc7663895 100644 --- a/python/semantic_kernel/kernel.py +++ b/python/semantic_kernel/kernel.py @@ -257,7 +257,10 @@ def _create_semantic_function( ) elif backend.backend_type == BackendType.OpenAI: if backend.open_ai is None: - raise ValueError("OpenAI configuration is missing") + raise ValueError( + "The OpenAI backend was requested, but is not configured: " + "unable to prepare the semantic function. " + ) function.set_chat_backend( lambda: OpenAIChatCompletion( @@ -290,7 +293,10 @@ def _create_semantic_function( if backend.backend_type == BackendType.AzureOpenAI: if backend.azure_open_ai is None: - raise ValueError("Azure OpenAI configuration is missing") + raise ValueError( + "The Azure OpenAI backend was requested, but is " + "not configured: unable to prepare the semantic function. " + ) function.set_ai_backend( lambda: AzureTextCompletion( backend.azure_open_ai.deployment_name, # type: ignore @@ -302,7 +308,10 @@ def _create_semantic_function( ) elif backend.backend_type == BackendType.OpenAI: if backend.open_ai is None: - raise ValueError("OpenAI configuration is missing") + raise ValueError( + "The OpenAI backend was requested, but is not configured: " + "unable to prepare the semantic function. " + ) function.set_ai_backend( lambda: OpenAITextCompletion( backend.open_ai.model_id, # type: ignore diff --git a/python/semantic_kernel/kernel_builder.py b/python/semantic_kernel/kernel_builder.py index 845b2e459521..28d7881b583e 100644 --- a/python/semantic_kernel/kernel_builder.py +++ b/python/semantic_kernel/kernel_builder.py @@ -31,25 +31,25 @@ def __init__( def with_configuration(self, config: KernelConfig) -> "KernelBuilder": if config is None: - raise ValueError("The configuration instance provided is None") + raise ValueError("The configuration instance cannot be `None`") self._config = config return self def with_memory(self, memory: SemanticTextMemoryBase) -> "KernelBuilder": if memory is None: - raise ValueError("The memory instance provided is None") + raise ValueError("The memory instance cannot be `None`") self._memory = memory return self def with_memory_storage(self, storage: MemoryStoreBase) -> "KernelBuilder": if storage is None: - raise ValueError("The memory storage instance provided is None") + raise ValueError("The memory storage instance cannot be `None`") self._memory_storage = storage return self def with_logger(self, log: Logger) -> "KernelBuilder": if log is None: - raise ValueError("The logger instance provided is None") + raise ValueError("The logger instance cannot be `None`") self._log = log return self diff --git a/python/semantic_kernel/kernel_extensions/memory_configuration.py b/python/semantic_kernel/kernel_extensions/memory_configuration.py index fd058656d713..e36a71a9bc82 100644 --- a/python/semantic_kernel/kernel_extensions/memory_configuration.py +++ b/python/semantic_kernel/kernel_extensions/memory_configuration.py @@ -23,7 +23,7 @@ def use_memory( if embeddings_generator is None: backend_label = kernel.config.default_embeddings_backend if not backend_label: - raise ValueError("The embedding backend label is empty") + raise ValueError("The embedding backend label cannot be `None` or empty") embeddings_backend_config = kernel.config.get_embeddings_backend(backend_label) if embeddings_backend_config is None: @@ -53,8 +53,8 @@ def use_memory( ) if storage is None: - raise ValueError("The storage instance provided is None") + raise ValueError("The storage instance provided cannot be `None`") if embeddings_generator is None: - raise ValueError("The embedding generator is None") + raise ValueError("The embedding generator cannot be `None`") kernel.register_memory(SemanticTextMemory(storage, embeddings_generator)) diff --git a/python/semantic_kernel/orchestration/context_variables.py b/python/semantic_kernel/orchestration/context_variables.py index bab33f414c61..2a1479267ee5 100644 --- a/python/semantic_kernel/orchestration/context_variables.py +++ b/python/semantic_kernel/orchestration/context_variables.py @@ -35,7 +35,7 @@ def merge_or_overwrite( def set(self, name: str, value: str) -> "ContextVariables": if not name: - raise ValueError("The variable name cannot be empty") + raise ValueError("The variable name cannot be `None` or empty") name = name.lower() if value is not None: @@ -58,7 +58,7 @@ def __getitem__(self, name: str) -> str: def __setitem__(self, name: str, value: str) -> None: if not name: - raise ValueError("The variable name cannot be empty") + raise ValueError("The variable name cannot be `None` or empty") name = name.lower() self._variables[name] = value diff --git a/python/semantic_kernel/orchestration/sk_function.py b/python/semantic_kernel/orchestration/sk_function.py index ba0d978e53f5..68c74cd7c6e3 100644 --- a/python/semantic_kernel/orchestration/sk_function.py +++ b/python/semantic_kernel/orchestration/sk_function.py @@ -43,7 +43,7 @@ class SKFunction(SKFunctionBase): @staticmethod def from_native_method(method, skill_name="", log=None) -> "SKFunction": if method is None: - raise ValueError("Method cannot be empty") + raise ValueError("Method cannot be `None`") assert method.__sk_function__ is not None, "Method is not a SK function" assert method.__sk_function_name__ is not None, "Method name is empty" @@ -89,11 +89,11 @@ def from_semantic_config( log: Optional[Logger] = None, ) -> "SKFunction": if function_config is None: - raise ValueError("Function configuration cannot be empty") + raise ValueError("Function configuration cannot be `None`") async def _local_func(client, request_settings, context): if client is None: - raise ValueError("AI LLM backend cannot be empty") + raise ValueError("AI LLM backend cannot be `None`") try: if function_config.has_chat_prompt: @@ -203,7 +203,7 @@ def set_ai_backend( self, ai_backend: Callable[[], TextCompletionClientBase] ) -> "SKFunction": if ai_backend is None: - raise ValueError("AI LLM backend factory cannot be empty") + raise ValueError("AI LLM backend factory cannot be `None`") self._verify_is_semantic() self._ai_backend = ai_backend() return self @@ -212,21 +212,21 @@ def set_chat_backend( self, chat_backend: Callable[[], ChatCompletionClientBase] ) -> "SKFunction": if chat_backend is None: - raise ValueError("Chat LLM backend factory cannot be empty") + raise ValueError("Chat LLM backend factory cannot be `None`") self._verify_is_semantic() self._chat_backend = chat_backend() return self def set_ai_configuration(self, settings: CompleteRequestSettings) -> "SKFunction": if settings is None: - raise ValueError("AI LLM request settings cannot be empty") + raise ValueError("AI LLM request settings cannot be `None`") self._verify_is_semantic() self._ai_request_settings = settings return self def set_chat_configuration(self, settings: ChatRequestSettings) -> "SKFunction": if settings is None: - raise ValueError("Chat LLM request settings cannot be empty") + raise ValueError("Chat LLM request settings cannot be `None`") self._verify_is_semantic() self._chat_request_settings = settings return self @@ -249,7 +249,7 @@ async def invoke_async( ) -> SKContext: if context is None: if self._skill_collection is None: - raise ValueError("Skill collection cannot be empty") + raise ValueError("Skill collection cannot be `None`") assert self._skill_collection is not None context = SKContext( diff --git a/python/semantic_kernel/skill_definition/skill_collection.py b/python/semantic_kernel/skill_definition/skill_collection.py index 51f178253cea..b4977ead4641 100644 --- a/python/semantic_kernel/skill_definition/skill_collection.py +++ b/python/semantic_kernel/skill_definition/skill_collection.py @@ -35,7 +35,7 @@ def __init__(self, log: Optional[Logger] = None) -> None: def add_semantic_function(self, function: "SKFunctionBase") -> None: if function is None: - raise ValueError("The function provided cannot be None") + raise ValueError("The function provided cannot be `None`") s_name, f_name = function.skill_name, function.name s_name, f_name = s_name.lower(), f_name.lower() @@ -47,7 +47,7 @@ def add_semantic_function(self, function: "SKFunctionBase") -> None: def add_native_function(self, function: "SKFunctionBase") -> None: if function is None: - raise ValueError("The function provided cannot be None") + raise ValueError("The function provided cannot be `None`") s_name, f_name = function.skill_name, function.name s_name, f_name = self._normalize_names(s_name, f_name, True) @@ -143,7 +143,7 @@ def _normalize_names( s_name = self.GLOBAL_SKILL if s_name is None: - raise ValueError("The skill name provided cannot be None") + raise ValueError("The skill name provided cannot be `None`") s_name, f_name = s_name.lower(), f_name.lower() return s_name, f_name diff --git a/python/semantic_kernel/utils/validation.py b/python/semantic_kernel/utils/validation.py index 3bc8914baa7f..bc508999dece 100644 --- a/python/semantic_kernel/utils/validation.py +++ b/python/semantic_kernel/utils/validation.py @@ -16,7 +16,7 @@ def validate_skill_name(value: Optional[str]) -> None: :raises ValueError: If the skill name is invalid. """ if not value: - raise ValueError("The skill name cannot be empty") + raise ValueError("The skill name cannot be `None` or empty") SKILL_NAME_REGEX = r"^[0-9A-Za-z_]*$" if not re_match(SKILL_NAME_REGEX, value): @@ -38,7 +38,7 @@ def validate_function_name(value: Optional[str]) -> None: :raises ValueError: If the function name is invalid. """ if not value: - raise ValueError("The function name cannot be empty") + raise ValueError("The function name cannot be `None` or empty") FUNCTION_NAME_REGEX = r"^[0-9A-Za-z_]*$" if not re_match(FUNCTION_NAME_REGEX, value): @@ -60,7 +60,7 @@ def validate_function_param_name(value: Optional[str]) -> None: :raises ValueError: If the function parameter name is invalid. """ if not value: - raise ValueError("The function parameter name cannot be empty") + raise ValueError("The function parameter name cannot be `None` or empty") FUNCTION_PARAM_NAME_REGEX = r"^[0-9A-Za-z_]*$" if not re_match(FUNCTION_PARAM_NAME_REGEX, value): From 61dead12c11bca66855b69b8f770f79596b87eb0 Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Mon, 3 Apr 2023 12:48:15 -0500 Subject: [PATCH 07/10] Fix this file: out of compliance w/ make pre-commit --- python/tests/test_config.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/python/tests/test_config.py b/python/tests/test_config.py index 0acb6c2e8898..1e3cf555300a 100644 --- a/python/tests/test_config.py +++ b/python/tests/test_config.py @@ -1,9 +1,11 @@ # Copyright (c) Microsoft. All rights reserved. import os -import pytest + import semantic_kernel as sk -import semantic_kernel.kernel_extensions.import_semantic_skill_from_directory as importer +from semantic_kernel.kernel_extensions.import_semantic_skill_from_directory import ( + import_semantic_skill_from_directory, +) def test_can_be_imported(): @@ -11,12 +13,16 @@ def test_can_be_imported(): kernel = sk.create_kernel() api_key = "test-api-key" org_id = "test-org-id" - kernel.config.add_openai_completion_backend("test-completion-backend", api_key, org_id) + kernel.config.add_openai_completion_backend( + "test-completion-backend", api_key, org_id + ) # import skills skills_directory = os.path.join(os.path.dirname(__file__), "test_skills") # path to skills directory - skill_config_dict = importer.import_semantic_skill_from_directory(kernel, skills_directory, "TestSkill") + skill_config_dict = import_semantic_skill_from_directory( + kernel, skills_directory, "TestSkill" + ) assert skill_config_dict is not None assert len(skill_config_dict) == 1 From d002e873402d3f56bac83e50745f31ce5420f975 Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Mon, 3 Apr 2023 19:46:57 -0500 Subject: [PATCH 08/10] Run linter/isort --- python/tests/unit/memory/test_volatile_memory_store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/tests/unit/memory/test_volatile_memory_store.py b/python/tests/unit/memory/test_volatile_memory_store.py index c40d569be2c9..7bd0a4f99375 100644 --- a/python/tests/unit/memory/test_volatile_memory_store.py +++ b/python/tests/unit/memory/test_volatile_memory_store.py @@ -1,5 +1,5 @@ import numpy as np -from pytest import raises, mark +from pytest import mark, raises from semantic_kernel.memory import VolatileMemoryStore From 480306189bfe5f50158bfff1ae7cbd16fc0ad5a1 Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Tue, 4 Apr 2023 11:20:01 -0500 Subject: [PATCH 09/10] Improve user-facing error messages --- python/semantic_kernel/utils/validation.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/python/semantic_kernel/utils/validation.py b/python/semantic_kernel/utils/validation.py index bc508999dece..a2807d61a68e 100644 --- a/python/semantic_kernel/utils/validation.py +++ b/python/semantic_kernel/utils/validation.py @@ -22,7 +22,8 @@ def validate_skill_name(value: Optional[str]) -> None: if not re_match(SKILL_NAME_REGEX, value): raise ValueError( f"Invalid skill name: {value}. Skill " - f"names must match the regex: {SKILL_NAME_REGEX}" + f"names may only contain ASCII letters, " + f"digits, and underscores." ) @@ -44,7 +45,8 @@ def validate_function_name(value: Optional[str]) -> None: if not re_match(FUNCTION_NAME_REGEX, value): raise ValueError( f"Invalid function name: {value}. Function " - f"names must match the regex: {FUNCTION_NAME_REGEX}" + f"names may only contain ASCII letters, " + f"digits, and underscores." ) @@ -66,5 +68,5 @@ def validate_function_param_name(value: Optional[str]) -> None: if not re_match(FUNCTION_PARAM_NAME_REGEX, value): raise ValueError( f"Invalid function parameter name: {value}. Function parameter " - f"names must match the regex: {FUNCTION_PARAM_NAME_REGEX}" + f"names may only contain ASCII letters, digits, and underscores." ) From 13842b2f324d14fab017e5c1f9e820cafc1e44f8 Mon Sep 17 00:00:00 2001 From: Jordan Henkel Date: Tue, 4 Apr 2023 17:05:16 -0500 Subject: [PATCH 10/10] Remove Verify again after merge --- python/.env.example | 2 +- .../semantic_kernel/kernel_extensions/import_skills.py | 9 ++++++--- .../kernel_extensions/inline_definition.py | 6 +++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/python/.env.example b/python/.env.example index 14a6df4ef320..946e6f0746d2 100644 --- a/python/.env.example +++ b/python/.env.example @@ -1,4 +1,4 @@ OPENAI_API_KEY="" OPENAI_ORG_ID="" AZURE_OPENAI_API_KEY="" -AZURE_OPENAI_ENDPOINT="" \ No newline at end of file +AZURE_OPENAI_ENDPOINT="" diff --git a/python/semantic_kernel/kernel_extensions/import_skills.py b/python/semantic_kernel/kernel_extensions/import_skills.py index 13fd5cd9ab83..06dda3077e70 100644 --- a/python/semantic_kernel/kernel_extensions/import_skills.py +++ b/python/semantic_kernel/kernel_extensions/import_skills.py @@ -4,7 +4,6 @@ import os from typing import Dict -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_extensions.extends_kernel import ExtendsKernel from semantic_kernel.orchestration.sk_function_base import SKFunctionBase from semantic_kernel.semantic_functions.prompt_template import PromptTemplate @@ -14,6 +13,7 @@ from semantic_kernel.semantic_functions.semantic_function_config import ( SemanticFunctionConfig, ) +from semantic_kernel.utils.validation import validate_skill_name class ImportSkills(ExtendsKernel): @@ -25,10 +25,13 @@ def import_semantic_skill_from_directory( kernel = self.kernel() - Verify.valid_skill_name(skill_directory_name) + validate_skill_name(skill_directory_name) + skill_directory = os.path.join(parent_directory, skill_directory_name) skill_directory = os.path.abspath(skill_directory) - Verify.directory_exists(skill_directory) + + if not os.path.exists(skill_directory): + raise ValueError(f"Skill directory does not exist: {skill_directory_name}") skill = {} diff --git a/python/semantic_kernel/kernel_extensions/inline_definition.py b/python/semantic_kernel/kernel_extensions/inline_definition.py index 5270027f5838..2160d06752e6 100644 --- a/python/semantic_kernel/kernel_extensions/inline_definition.py +++ b/python/semantic_kernel/kernel_extensions/inline_definition.py @@ -3,7 +3,6 @@ from typing import TYPE_CHECKING, List, Optional from uuid import uuid4 -from semantic_kernel.diagnostics.verify import Verify from semantic_kernel.kernel_extensions.extends_kernel import ExtendsKernel from semantic_kernel.semantic_functions.prompt_template import PromptTemplate from semantic_kernel.semantic_functions.prompt_template_config import ( @@ -12,6 +11,7 @@ from semantic_kernel.semantic_functions.semantic_function_config import ( SemanticFunctionConfig, ) +from semantic_kernel.utils.validation import validate_function_name, validate_skill_name if TYPE_CHECKING: from semantic_kernel.orchestration.sk_function_base import SKFunctionBase @@ -56,9 +56,9 @@ def create_semantic_function( ), ) - Verify.valid_function_name(function_name) + validate_function_name(function_name) if skill_name is not None: - Verify.valid_skill_name(skill_name) + validate_skill_name(skill_name) template = PromptTemplate( prompt_template, kernel.prompt_template_engine, config