codenohup commented on code in PR #67: URL: https://github.com/apache/flink-agents/pull/67#discussion_r2209627514
########## python/flink_agents/api/memoryobject.py: ########## @@ -0,0 +1,116 @@ +################################################################################ +# 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, Dict, List + +from pydantic import BaseModel + +class MemoryObject(BaseModel, ABC): + """ + Representation of an object in the short-term memory. + + A direct field is a field which stores concrete data directly, while an indirect filed is + just a "prefix" which represents a nested object. + Fields can be accessed using an absolute or relative path. + """ + + @abstractmethod + def get(self, path: str) -> Any: + """ + Get the value of a (direct or indirect) field in the object. + + Parameters + ---------- + path: str + Relative path from the current object to the target field. + + Returns: + ------- + Any + The value of the field. If the field is an object, another MemoryObject will be returned. Review Comment: not clear, I don't quite understand it ########## python/flink_agents/api/memoryobject.py: ########## @@ -0,0 +1,116 @@ +################################################################################ +# 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, Dict, List + +from pydantic import BaseModel + +class MemoryObject(BaseModel, ABC): + """ + Representation of an object in the short-term memory. + + A direct field is a field which stores concrete data directly, while an indirect filed is + just a "prefix" which represents a nested object. + Fields can be accessed using an absolute or relative path. + """ + + @abstractmethod + def get(self, path: str) -> Any: + """ + Get the value of a (direct or indirect) field in the object. + + Parameters + ---------- + path: str + Relative path from the current object to the target field. + + Returns: + ------- + Any + The value of the field. If the field is an object, another MemoryObject will be returned. + If the field doesn't exist, returns None. + """ + + + @abstractmethod + def set(self, path: str, value: Any): + """ + Set the value of a (direct or indirect) field in the object. + This will also create the intermediate objects if not exist. + + Parameters + ---------- + path: str + Relative path from the current object to the target field. + value: Any + New value of the field. The type of the value must be either a primary type, or MemoryObject. Review Comment: Confirm why we need to set a memory object through the `set` method instead of through `new_object` ########## python/flink_agents/runtime/local_memory_object.py: ########## @@ -0,0 +1,219 @@ +################################################################################ +# 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 typing import Any, Dict, List, ClassVar + +from flink_agents.api.memoryobject import MemoryObject + +class LocalMemoryObject(MemoryObject): + """ + LocalMemoryObject: Flattened hierarchical key-value store for local python execution. + + Each object keeps a prefix to represent its path. + """ + ROOT_KEY: ClassVar[str] = "" + SEPARATOR: ClassVar[str] = "." + NESTED_MARK: ClassVar[str] = "NestedObject" + + _store: dict[str, Any] + _prefix: str + + def __init__(self, store: Dict[str, Any], prefix: str = ROOT_KEY): + super().__init__() + self._store = store if store is not None else {} + self._prefix = prefix + + if self.ROOT_KEY not in self._store: + self._store[self.ROOT_KEY] = _ObjMarker() + + def get(self, path: str) -> Any: + """ + Get the value of a (direct or indirect) field in the object. + + Parameters + ---------- + path: str + Relative path from the current object to the target field. + + Returns: + ------- + Any + The value of the field. If the field is an indirect field, another MemoryObject will be returned. + If the field doesn't exist, returns None. Review Comment: There are three return values: value, MemoryObject, and None. They should be clearly stated. ########## python/flink_agents/examples/agent_example.py: ########## @@ -40,16 +40,33 @@ class MyAgent(Agent): @staticmethod def first_action(event: Event, ctx: RunnerContext): # noqa D102 input = event.input - content = input + " first_action" + memory = ctx.get_short_term_memory() + + current = memory.get("counter") or 0 Review Comment: It seems that this _counter_ is counting actions. You can consider changing the name to _action_counter_. ########## python/flink_agents/examples/agent_example.py: ########## @@ -40,16 +40,33 @@ class MyAgent(Agent): @staticmethod def first_action(event: Event, ctx: RunnerContext): # noqa D102 input = event.input - content = input + " first_action" + memory = ctx.get_short_term_memory() + + current = memory.get("counter") or 0 Review Comment: ```suggestion current_count = memory.get("counter") or 0 ``` ########## python/flink_agents/api/memoryobject.py: ########## @@ -0,0 +1,116 @@ +################################################################################ +# 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, Dict, List + +from pydantic import BaseModel + +class MemoryObject(BaseModel, ABC): + """ + Representation of an object in the short-term memory. + + A direct field is a field which stores concrete data directly, while an indirect filed is + just a "prefix" which represents a nested object. + Fields can be accessed using an absolute or relative path. + """ + + @abstractmethod + def get(self, path: str) -> Any: + """ + Get the value of a (direct or indirect) field in the object. + + Parameters + ---------- + path: str + Relative path from the current object to the target field. + + Returns: + ------- + Any + The value of the field. If the field is an object, another MemoryObject will be returned. + If the field doesn't exist, returns None. + """ + + + @abstractmethod + def set(self, path: str, value: Any): + """ + Set the value of a (direct or indirect) field in the object. + This will also create the intermediate objects if not exist. + + Parameters + ---------- + path: str + Relative path from the current object to the target field. + value: Any + New value of the field. The type of the value must be either a primary type, or MemoryObject. + """ + + @abstractmethod + def new_object(self, path: str) -> 'MemoryObject': + """ + Create a new object as the value of a (direct or indirect) field in the object. Review Comment: Why can we create a indirect field through `new_object`? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
