Ver código fonte

Merge pull request #2649 from Avaiga/refactor/#2616-make-read-write-query-optional-in-sql-dn

Fix/#2616 - Make read_query and write_query_builder optional for SQLDataNode
Đỗ Trường Giang 2 dias atrás
pai
commit
58fe2a9aaf

+ 23 - 5
taipy/core/config/checkers/_data_node_config_checker.py

@@ -49,6 +49,7 @@ class _DataNodeConfigChecker(_ConfigChecker):
             self._check_required_properties(data_node_config_id, data_node_config)
             self._check_property_types(data_node_config_id, data_node_config)
             self._check_generic_read_write_fct_and_args(data_node_config_id, data_node_config)
+            self._check_sql_read_write_query(data_node_config_id, data_node_config)
             self._check_exposed_type(data_node_config_id, data_node_config)
         return self._collector
 
@@ -111,8 +112,6 @@ class _DataNodeConfigChecker(_ConfigChecker):
                         required_properties = [
                             DataNodeConfig._REQUIRED_DB_NAME_SQL_PROPERTY,
                             DataNodeConfig._REQUIRED_DB_ENGINE_SQL_PROPERTY,
-                            DataNodeConfig._REQUIRED_READ_QUERY_SQL_PROPERTY,
-                            DataNodeConfig._REQUIRED_WRITE_QUERY_BUILDER_SQL_PROPERTY,
                         ]
                     else:
                         required_properties = [
@@ -120,8 +119,6 @@ class _DataNodeConfigChecker(_ConfigChecker):
                             DataNodeConfig._OPTIONAL_DB_PASSWORD_SQL_PROPERTY,
                             DataNodeConfig._REQUIRED_DB_NAME_SQL_PROPERTY,
                             DataNodeConfig._REQUIRED_DB_ENGINE_SQL_PROPERTY,
-                            DataNodeConfig._REQUIRED_READ_QUERY_SQL_PROPERTY,
-                            DataNodeConfig._REQUIRED_WRITE_QUERY_BUILDER_SQL_PROPERTY,
                         ]
                     return required_properties
         if storage_type == DataNodeConfig._STORAGE_TYPE_VALUE_SQL_TABLE:
@@ -144,7 +141,7 @@ class _DataNodeConfigChecker(_ConfigChecker):
                     return required_properties
         return []
 
-    def __storage_type_specific_required_properties(self, storage_type: str, dn_config_properties: Dict) -> List:
+    def __storage_type_specific_required_properties(self, storage_type: str, dn_config_properties) -> List:
         if storage_type in (DataNodeConfig._STORAGE_TYPE_VALUE_SQL_TABLE, DataNodeConfig._STORAGE_TYPE_VALUE_SQL):
             return self.__get_sql_required_properties(storage_type, dn_config_properties)
         return []
@@ -212,6 +209,27 @@ class _DataNodeConfigChecker(_ConfigChecker):
                     f"DataNodeConfig `{data_node_config_id}` must be populated with a Callable function.",
                 )
 
+    def _check_sql_read_write_query(self, data_node_config_id: str, data_node_config: DataNodeConfig):
+        if data_node_config.storage_type != DataNodeConfig._STORAGE_TYPE_VALUE_SQL:
+            return
+
+        if data_node_config_id != DataNodeConfig._DEFAULT_KEY:
+            properties_to_check_at_least_one = [
+                DataNodeConfig._OPTIONAL_READ_QUERY_SQL_PROPERTY,
+                DataNodeConfig._OPTIONAL_WRITE_QUERY_BUILDER_SQL_PROPERTY,
+            ]
+            if not any(
+                data_node_config.properties and prop_key in data_node_config.properties
+                for prop_key in properties_to_check_at_least_one
+            ):
+                self._error(
+                    ", ".join(properties_to_check_at_least_one),
+                    None,
+                    f"Either `{DataNodeConfig._OPTIONAL_READ_QUERY_SQL_PROPERTY}` (str) or "
+                    f"`{DataNodeConfig._OPTIONAL_WRITE_QUERY_BUILDER_SQL_PROPERTY}` (Callable) of "
+                    f"DataNodeConfig `{data_node_config_id}` must be provided.",
+                )
+
     def _check_property_types(self, data_node_config_id: str, data_node_config: DataNodeConfig):
         if property_types := data_node_config._PROPERTIES_TYPES.get(data_node_config.storage_type):
             for prop_key, prop_type in property_types.items():

+ 8 - 8
taipy/core/config/data_node_config.py

@@ -120,8 +120,8 @@ class DataNodeConfig(Section):
     # SQL_TABLE
     _REQUIRED_TABLE_NAME_SQL_TABLE_PROPERTY = "table_name"
     # SQL
-    _REQUIRED_READ_QUERY_SQL_PROPERTY = "read_query"
-    _REQUIRED_WRITE_QUERY_BUILDER_SQL_PROPERTY = "write_query_builder"
+    _OPTIONAL_READ_QUERY_SQL_PROPERTY = "read_query"
+    _OPTIONAL_WRITE_QUERY_BUILDER_SQL_PROPERTY = "write_query_builder"
     _OPTIONAL_APPEND_QUERY_BUILDER_SQL_PROPERTY = "append_query_builder"
     # MONGO
     _REQUIRED_DB_NAME_MONGO_PROPERTY = "db_name"
@@ -167,8 +167,8 @@ class DataNodeConfig(Section):
         _STORAGE_TYPE_VALUE_SQL: {
             _REQUIRED_DB_NAME_SQL_PROPERTY: str,
             _REQUIRED_DB_ENGINE_SQL_PROPERTY: str,
-            _REQUIRED_READ_QUERY_SQL_PROPERTY: str,
-            _REQUIRED_WRITE_QUERY_BUILDER_SQL_PROPERTY: Callable,
+            _OPTIONAL_READ_QUERY_SQL_PROPERTY: str,
+            _OPTIONAL_WRITE_QUERY_BUILDER_SQL_PROPERTY: Callable,
             _OPTIONAL_APPEND_QUERY_BUILDER_SQL_PROPERTY: Callable,
             _OPTIONAL_DB_USERNAME_SQL_PROPERTY: str,
             _OPTIONAL_DB_PASSWORD_SQL_PROPERTY: str,
@@ -260,8 +260,6 @@ class DataNodeConfig(Section):
         _STORAGE_TYPE_VALUE_SQL: [
             _REQUIRED_DB_NAME_SQL_PROPERTY,
             _REQUIRED_DB_ENGINE_SQL_PROPERTY,
-            _REQUIRED_READ_QUERY_SQL_PROPERTY,
-            _REQUIRED_WRITE_QUERY_BUILDER_SQL_PROPERTY,
         ],
         _STORAGE_TYPE_VALUE_MONGO_COLLECTION: [
             _REQUIRED_DB_NAME_MONGO_PROPERTY,
@@ -1088,11 +1086,13 @@ class DataNodeConfig(Section):
             {
                 cls._REQUIRED_DB_NAME_SQL_PROPERTY: db_name,
                 cls._REQUIRED_DB_ENGINE_SQL_PROPERTY: db_engine,
-                cls._REQUIRED_READ_QUERY_SQL_PROPERTY: read_query,
-                cls._REQUIRED_WRITE_QUERY_BUILDER_SQL_PROPERTY: write_query_builder,
             }
         )
 
+        if read_query is not None:
+            properties[cls._OPTIONAL_READ_QUERY_SQL_PROPERTY] = read_query
+        if write_query_builder is not None:
+            properties[cls._OPTIONAL_WRITE_QUERY_BUILDER_SQL_PROPERTY] = write_query_builder
         if append_query_builder is not None:
             properties[cls._OPTIONAL_APPEND_QUERY_BUILDER_SQL_PROPERTY] = append_query_builder
         if db_username is not None:

+ 16 - 14
taipy/core/data/sql.py

@@ -16,7 +16,7 @@ from sqlalchemy import text
 
 from .._version._version_manager_factory import _VersionManagerFactory
 from ..common.scope import Scope
-from ..exceptions.exceptions import MissingAppendQueryBuilder, MissingRequiredProperty
+from ..exceptions.exceptions import MissingAppendQueryBuilder, MissingReadQuery, MissingWriteQueryBuilder
 from ._abstract_sql import _AbstractSQLDataNode
 from .data_node_id import DataNodeId, Edit
 
@@ -31,13 +31,6 @@ class SQLDataNode(_AbstractSQLDataNode):
     - *db_name* (`str`): The database name, or the name of the SQLite database file.
     - *db_engine* (`str`): The database engine. Possible values are *sqlite*, *mssql*,
         *mysql*, or *postgresql*.
-    - *read_query* (`str`): The SQL query string used to read the data from the database.
-    - *write_query_builder* `(Callable)`: A callback function that takes the data as an input
-        parameter and returns a list of SQL queries to be executed when writing data to the data
-        node.
-    - *append_query_builder* (`Callable`): A callback function that takes the data as an input
-        parameter and returns a list of SQL queries to be executed when appending data to the
-        data node.
     - *db_username* (`str`): The database username.
     - *db_password* (`str`): The database password.
     - *db_host* (`str`): The database host. The default value is *localhost*.
@@ -46,6 +39,11 @@ class SQLDataNode(_AbstractSQLDataNode):
 
     The *properties* attribute can also contain the following optional entries:
 
+    - *read_query* (`str`): The SQL query string used to read the data from the database.
+    - *write_query_builder* `(Callable)`: A callback function that takes the data as an input parameter
+        and returns a list of SQL queries to be executed when writing data to the data node.
+    - *append_query_builder* (`Callable`): A callback function that takes the data as an input parameter
+        and returns a list of SQL queries to be executed when appending data to the data node.
     - *sqlite_folder_path* (str): The path to the folder that contains SQLite file. The default value
         is the current working folder.
     - *sqlite_file_extension* (str): The filename extension of the SQLite file. The default value is ".db".
@@ -76,10 +74,6 @@ class SQLDataNode(_AbstractSQLDataNode):
     ) -> None:
         if properties is None:
             properties = {}
-        if properties.get(self.__READ_QUERY_KEY) is None:
-            raise MissingRequiredProperty(f"Property {self.__READ_QUERY_KEY} is not informed and is required.")
-        if properties.get(self._WRITE_QUERY_BUILDER_KEY) is None:
-            raise MissingRequiredProperty(f"Property {self._WRITE_QUERY_BUILDER_KEY} is not informed and is required.")
 
         super().__init__(
             config_id,
@@ -111,7 +105,11 @@ class SQLDataNode(_AbstractSQLDataNode):
         return cls.__STORAGE_TYPE
 
     def _get_base_read_query(self) -> str:
-        return self.properties.get(self.__READ_QUERY_KEY)
+        read_query = self.properties.get(self.__READ_QUERY_KEY)
+        if not read_query:
+            raise MissingReadQuery
+
+        return read_query
 
     def _do_append(self, data, engine, connection) -> None:
         append_query_builder_fct = self.properties.get(self._APPEND_QUERY_BUILDER_KEY)
@@ -122,7 +120,11 @@ class SQLDataNode(_AbstractSQLDataNode):
         self.__execute_queries(queries, connection)
 
     def _do_write(self, data, engine, connection) -> None:
-        queries = self.properties.get(self._WRITE_QUERY_BUILDER_KEY)(data)
+        write_query_builder_fct = self.properties.get(self._WRITE_QUERY_BUILDER_KEY)
+        if not write_query_builder_fct:
+            raise MissingWriteQueryBuilder
+
+        queries = write_query_builder_fct(data)
         self.__execute_queries(queries, connection)
 
     def __execute_queries(self, queries, connection) -> None:

+ 10 - 2
taipy/core/exceptions/exceptions.py

@@ -75,12 +75,20 @@ class UnknownDatabaseEngine(Exception):
     """Raised if the database engine is not known when creating a connection with a SQLDataNode."""
 
 
+class MissingReadQuery(Exception):
+    """Raised if read_query attribute is not provided when reading data from a SQLDataNode."""
+
+
+class MissingWriteQueryBuilder(Exception):
+    """Raised if write_query_builder attribute is not provided when writing data to a SQLDataNode."""
+
+
 class MissingAppendQueryBuilder(Exception):
-    """Raised if no append query build is provided when appending data to a SQLDataNode."""
+    """Raised if append_query_builder attribute is not provided when appending data to a SQLDataNode."""
 
 
 class UnknownParquetEngine(Exception):
-    """Raised if the parquet engine is not known or not supported when create a ParquetDataNode."""
+    """Raised if the parquet engine is not known or not supported when creating a ParquetDataNode."""
 
 
 class UnknownCompressionAlgorithm(Exception):

+ 39 - 4
tests/core/config/checkers/test_data_node_config_checker.py

@@ -192,12 +192,11 @@ class TestDataNodeConfigChecker:
         with pytest.raises(SystemExit):
             Config._collector = IssueCollector()
             Config.check()
-        assert len(Config._collector.errors) == 4
+        assert len(Config._collector.errors) == 3
         expected_error_messages = [
             "DataNodeConfig `new` is missing the required property `db_name` for type `sql`.",
             "DataNodeConfig `new` is missing the required property `db_engine` for type `sql`.",
-            "DataNodeConfig `new` is missing the required property `read_query` for type `sql`.",
-            "DataNodeConfig `new` is missing the required property `write_query_builder` for type `sql`.",
+            "Either `read_query` (str) or `write_query_builder` (Callable) of DataNodeConfig `new` must be provided.",
         ]
         assert all(message in caplog.text for message in expected_error_messages)
 
@@ -389,6 +388,31 @@ class TestDataNodeConfigChecker:
         Config.check()
         assert len(Config._collector.errors) == 0
 
+        config._sections[DataNodeConfig.name]["new"].storage_type = "sql"
+        config._sections[DataNodeConfig.name]["new"].properties = {
+            "db_username": "foo",
+            "db_password": "foo",
+            "db_name": "foo",
+            "db_engine": "foo",
+        }
+        with pytest.raises(SystemExit):
+            Config._collector = IssueCollector()
+            Config.check()
+        assert len(Config._collector.errors) == 1
+
+        config._sections[DataNodeConfig.name]["new"].storage_type = "sql"
+        config._sections[DataNodeConfig.name]["new"].properties = {
+            "db_username": "foo",
+            "db_password": "foo",
+            "db_name": "foo",
+            "db_engine": "foo",
+            "read_query": "foo",
+            "write_query_builder": print,
+        }
+        Config._collector = IssueCollector()
+        Config.check()
+        assert len(Config._collector.errors) == 0
+
         config._sections[DataNodeConfig.name]["new"].storage_type = "sql"
         config._sections[DataNodeConfig.name]["new"].properties = {
             "db_username": "foo",
@@ -396,6 +420,17 @@ class TestDataNodeConfigChecker:
             "db_name": "foo",
             "db_engine": "foo",
             "read_query": "foo",
+        }
+        Config._collector = IssueCollector()
+        Config.check()
+        assert len(Config._collector.errors) == 0
+
+        config._sections[DataNodeConfig.name]["new"].storage_type = "sql"
+        config._sections[DataNodeConfig.name]["new"].properties = {
+            "db_username": "foo",
+            "db_password": "foo",
+            "db_name": "foo",
+            "db_engine": "foo",
             "write_query_builder": print,
         }
         Config._collector = IssueCollector()
@@ -725,7 +760,7 @@ class TestDataNodeConfigChecker:
         Config.check()
         assert len(Config._collector.errors) == 0
 
-        config._sections[DataNodeConfig.name]["default"].properties = {"default_data": 1.}
+        config._sections[DataNodeConfig.name]["default"].properties = {"default_data": 1.0}
         Config._collector = IssueCollector()
         Config.check()
         assert len(Config._collector.errors) == 0

+ 39 - 1
tests/core/data/test_sql_data_node.py

@@ -23,7 +23,13 @@ from taipy.core.data._data_manager_factory import _DataManagerFactory
 from taipy.core.data.data_node_id import DataNodeId
 from taipy.core.data.operator import JoinOperator, Operator
 from taipy.core.data.sql import SQLDataNode
-from taipy.core.exceptions.exceptions import MissingAppendQueryBuilder, MissingRequiredProperty, UnknownDatabaseEngine
+from taipy.core.exceptions.exceptions import (
+    MissingAppendQueryBuilder,
+    MissingReadQuery,
+    MissingRequiredProperty,
+    MissingWriteQueryBuilder,
+    UnknownDatabaseEngine,
+)
 
 
 class MyCustomObject:
@@ -223,6 +229,22 @@ class TestSQLDataNode:
             assert len(engine_mock.mock_calls[4].args) == 1
             assert engine_mock.mock_calls[4].args[0].text == "DELETE FROM example"
 
+    @pytest.mark.parametrize("properties", __sql_properties)
+    def test_write_only_datanode(self, properties):
+        custom_properties = properties.copy()
+        custom_properties.pop("read_query")
+        dn = SQLDataNode("foo_bar", Scope.SCENARIO, properties=custom_properties)
+        _DataManagerFactory._build_manager()._repository._save(dn)
+
+        with patch("sqlalchemy.engine.Engine.connect"):
+            dn.write(pd.DataFrame({"foo": [1, 2, 3], "bar": [4, 5, 6]}))
+
+        with pytest.raises(MissingReadQuery):
+            dn.read()
+
+        with pytest.raises(MissingAppendQueryBuilder):
+            dn.append(pd.DataFrame({"foo": [1, 2, 3], "bar": [4, 5, 6]}))
+
     @pytest.mark.parametrize(
         "tmp_sqlite_path",
         [
@@ -246,6 +268,22 @@ class TestSQLDataNode:
         data = dn.read()
         assert data.equals(pd.DataFrame([{"foo": 1, "bar": 2}, {"foo": 3, "bar": 4}]))
 
+    @pytest.mark.parametrize("properties", __sql_properties)
+    def test_read_only_datanode(self, properties):
+        custom_properties = properties.copy()
+        custom_properties.pop("write_query_builder")
+        dn = SQLDataNode("foo_bar", Scope.SCENARIO, properties=custom_properties)
+        _DataManagerFactory._build_manager()._repository._save(dn)
+
+        with patch("sqlalchemy.engine.Engine.connect"):
+            dn.read()
+
+        with pytest.raises(MissingWriteQueryBuilder):
+            dn.write(pd.DataFrame({"foo": [1, 2, 3], "bar": [4, 5, 6]}))
+
+        with pytest.raises(MissingAppendQueryBuilder):
+            dn.append(pd.DataFrame({"foo": [1, 2, 3], "bar": [4, 5, 6]}))
+
     def test_sqlite_append_pandas(self, tmp_sqlite_sqlite3_file_path):
         folder_path, db_name, file_extension = tmp_sqlite_sqlite3_file_path
         properties = {