alnzng commented on code in PR #122: URL: https://github.com/apache/flink-agents/pull/122#discussion_r2289962895
########## python/flink_agents/api/configuration.py: ########## @@ -0,0 +1,148 @@ +################################################################################ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################# +from abc import ABC, abstractmethod +from typing import Any, Type + + +class ConfigOption: + """A configuration option defines a configuration key with its type and default + value. + + Args: + key: The configuration key name + type: The expected type of the configuration value (int, float, str, or bool) + default: The default value for this configuration option + """ + + def __init__(self, key: str, type: Type[Any], default: Any) -> None: + """Initialize a configuration option.""" + self._key = key + self._type = type + self._default_value = default + def key(self) -> str: + """Gets the configuration key.""" + return self._key + + def default_value(self) -> Any: + """Returns the default value.""" + return self._default_value + +class WritableConfiguration(ABC): + """Abstract base class providing write access to a configuration object. + + This class enables modification of configuration settings. + """ + + @abstractmethod + def set_str(self, key: str, value: str) -> None: + """Set the string configuration value using the key. + + Args: + key: The configuration key to set + value: The string value to set for the key + """ + + @abstractmethod + def set_int(self, key: str, value: int) -> None: + """Set the int configuration value using the key. + + Args: + key: The configuration key to set + value: The integer value to set for the key + """ + + @abstractmethod + def set_float(self, key: str, value: float) -> None: + """Set the float configuration value using the key. + + Args: + key: The configuration key to set + value: The float value to set for the key + """ + + @abstractmethod + def set_bool(self, key: str, value: bool) -> None: # noqa: FBT001 + """Set the boolean configuration value using the key. + + Args: + key: The configuration key to set + value: The boolean value to set for the key + """ + + @abstractmethod + def set(self, option: ConfigOption, value: Any) -> None: + """Set the configuration value using the ConfigOption. + + Args: + option: The config option to set + value: The boolean value to set for the key Review Comment: remove `boolean ` ########## python/flink_agents/runtime/remote_execution_environment.py: ########## @@ -133,10 +138,26 @@ class RemoteExecutionEnvironment(AgentsExecutionEnvironment): """Implementation of AgentsExecutionEnvironment for execution with DataStream.""" __env: StreamExecutionEnvironment + __config: LocalConfiguration def __init__(self, env: StreamExecutionEnvironment) -> None: """Init method of RemoteExecutionEnvironment.""" self.__env = env + self.__config = LocalConfiguration() Review Comment: Seems both RemoteExecutionEnvironment and LocalExecutionEnvironment loads `LocalConfiguration`, is it expected? just from `LocalConfiguration` literately, it should be used for LocalExecutionEnvironment only. ########## python/flink_agents/api/configuration.py: ########## @@ -0,0 +1,148 @@ +################################################################################ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################# +from abc import ABC, abstractmethod +from typing import Any, Type + + +class ConfigOption: + """A configuration option defines a configuration key with its type and default + value. + + Args: + key: The configuration key name + type: The expected type of the configuration value (int, float, str, or bool) + default: The default value for this configuration option + """ + + def __init__(self, key: str, type: Type[Any], default: Any) -> None: + """Initialize a configuration option.""" + self._key = key + self._type = type + self._default_value = default + def key(self) -> str: + """Gets the configuration key.""" + return self._key + + def default_value(self) -> Any: + """Returns the default value.""" + return self._default_value + +class WritableConfiguration(ABC): + """Abstract base class providing write access to a configuration object. + + This class enables modification of configuration settings. + """ + + @abstractmethod + def set_str(self, key: str, value: str) -> None: + """Set the string configuration value using the key. + + Args: + key: The configuration key to set + value: The string value to set for the key + """ + + @abstractmethod + def set_int(self, key: str, value: int) -> None: + """Set the int configuration value using the key. + + Args: + key: The configuration key to set + value: The integer value to set for the key + """ + + @abstractmethod + def set_float(self, key: str, value: float) -> None: + """Set the float configuration value using the key. + + Args: + key: The configuration key to set + value: The float value to set for the key + """ + + @abstractmethod + def set_bool(self, key: str, value: bool) -> None: # noqa: FBT001 + """Set the boolean configuration value using the key. + + Args: + key: The configuration key to set + value: The boolean value to set for the key + """ + + @abstractmethod + def set(self, option: ConfigOption, value: Any) -> None: Review Comment: Should we support `get(option: ConfigOption) -> Any` method in ReadableConfiguration? ########## python/flink_agents/runtime/remote_execution_environment.py: ########## @@ -133,10 +138,26 @@ class RemoteExecutionEnvironment(AgentsExecutionEnvironment): """Implementation of AgentsExecutionEnvironment for execution with DataStream.""" __env: StreamExecutionEnvironment + __config: LocalConfiguration def __init__(self, env: StreamExecutionEnvironment) -> None: """Init method of RemoteExecutionEnvironment.""" self.__env = env + self.__config = LocalConfiguration() + flink_conf_dir = os.environ.get("FLINK_CONF_DIR") + if flink_conf_dir is not None: + config_dir = Path(flink_conf_dir) / "config.yaml" Review Comment: Does this `config.yaml` mean that flink-agents requires Flink 2.0+? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
